On 14/11/17 14:38, Victor Toso wrote: > 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. > Now I got it, you were talking about the other function, but commented on this block of code in remote_viewer_session_connected(), thus my confusion. I will change the check as suggested. Thanks, Eduardo. > 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 -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etrunko@xxxxxxxxxx _______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
