Hi, On Tue, Nov 14, 2017 at 02:14:25PM -0200, Eduardo Lima (Etrunko) wrote: > On 14/11/17 13:47, Victor Toso wrote: > > On Thu, Oct 26, 2017 at 03:39:31PM +0200, Eduardo Lima (Etrunko) wrote: > >> When connecting to a VM via oVirt instance, the original uri can not be > >> retrieved using virt_viewer_session_get_uri(). Consequently, it was > >> never saved, even though the connection succeeds and the actual callback > >> for "session-connected" signal, which saves the URI, is invoked. > >> > >> To solve this problem, we always pass a copy of the guri as user-data > >> parameter for the callback, and if the call to > >> virt_viewer_session_get_uri() returns NULL, the parameter is used > >> instead. > >> > >> Resolves: https://bugzilla.redhat.com/1459792 > >> > >> Signed-off-by: Eduardo Lima (Etrunko) <etrunko@xxxxxxxxxx> > >> --- > >> src/remote-viewer.c | 18 +++++++++--------- > >> 1 file changed, 9 insertions(+), 9 deletions(-) > >> > >> diff --git a/src/remote-viewer.c b/src/remote-viewer.c > >> index fb5376c..58dc04f 100644 > >> --- a/src/remote-viewer.c > >> +++ b/src/remote-viewer.c > >> @@ -85,10 +85,6 @@ static void spice_foreign_menu_updated(RemoteViewer *self); > >> static void foreign_menu_title_changed(SpiceCtrlForeignMenu *menu, GParamSpec *pspec, RemoteViewer *self); > >> #endif > >> > >> -static gboolean > >> -remote_viewer_initial_connect(RemoteViewer *self, const gchar *type, > >> - VirtViewerFile *vvfile, GError **error); > >> - > >> static void > >> remote_viewer_dispose (GObject *object) > >> { > >> @@ -1075,17 +1071,21 @@ remote_viewer_recent_add(gchar *uri, const gchar *mime_type) > >> > >> static void > >> remote_viewer_session_connected(VirtViewerSession *session, > >> - VirtViewerApp *self G_GNUC_UNUSED) > >> + gchar *guri) > >> { > >> gchar *uri = virt_viewer_session_get_uri(session); > >> const gchar *mime = virt_viewer_session_mime_type(session); > >> > >> + if (uri == NULL) > >> + uri = g_strdup(guri); > >> + > >> remote_viewer_recent_add(uri, mime); > > > > Could you also change the if (uri == NULL) in above function to a > > g_return_if_fail(uri != NULL) as it should not happen anymore... > > > > I don't really get what you mean here, as I explained on the commit > message, uri *can* be NULL. In this case, we will use the guri, which is > now passed as user_data argument to the callback. You are right, you just did not get what I mean here. If virt_viewer_session_get_uri() returns NULL we use the guri which *cannot be null* - That means that we can change the check in remote_viewer_recent_add() from if (uri == NULL) return; to g_return_if_fail() instead. This is more like a suggestion. Another possibility is just to remove the check (in remote_viewer_recent_add()) entirely) as we should have critical messages elsewhere if guri is null. Cheers, > > > Either way, > > Reviewed-by: Victor Toso <victortoso@xxxxxxxxxx> > > > >> g_free(uri); > >> + g_free(guri); > >> } > >> > >> static gboolean > >> -remote_viewer_initial_connect(RemoteViewer *self, const gchar *type, > >> +remote_viewer_initial_connect(RemoteViewer *self, const gchar *type, const gchar *guri, > >> VirtViewerFile *vvfile, GError **error) > >> { > >> VirtViewerApp *app = VIRT_VIEWER_APP(self); > >> @@ -1093,8 +1093,9 @@ remote_viewer_initial_connect(RemoteViewer *self, const gchar *type, > >> if (!virt_viewer_app_create_session(app, type, error)) > >> return FALSE; > >> > >> + > >> g_signal_connect(virt_viewer_app_get_session(app), "session-connected", > >> - G_CALLBACK(remote_viewer_session_connected), app); > >> + G_CALLBACK(remote_viewer_session_connected), g_strdup(guri)); > >> > >> virt_viewer_session_set_file(virt_viewer_app_get_session(app), vvfile); > >> #ifdef HAVE_OVIRT > >> @@ -1200,12 +1201,11 @@ retry_dialog: > >> } else > >> #endif > >> { > >> - if (!remote_viewer_initial_connect(self, type, vvfile, &error)) > >> + if (!remote_viewer_initial_connect(self, type, guri, vvfile, &error)) > >> goto cleanup; > >> } > >> } > >> > >> - > >> ret = VIRT_VIEWER_APP_CLASS(remote_viewer_parent_class)->start(app, &error); > >> > >> cleanup: > >> -- > >> 2.13.6 > >> > >> _______________________________________________ > >> virt-tools-list mailing list > >> virt-tools-list@xxxxxxxxxx > >> https://www.redhat.com/mailman/listinfo/virt-tools-list > > > -- > Eduardo de Barros Lima (Etrunko) > Software Engineer - RedHat > etrunko@xxxxxxxxxx
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
