[virt-tools-list] [PATCH v2] remote-viewer: fix free on dangling pointer

[Victor Toso]

Hi,

On Tue, Oct 01, 2019 at 11:17:22AM -0300, Eduardo Lima (Etrunko) wrote:
> On 10/1/19 7:09 AM, Victor Toso wrote:
> > > Patch builds now, but I started getting some warning when
> > > connecting to ovirt:// uri.
> > > 
> > > (remote-viewer:16940): virt-viewer-CRITICAL **: 11:00:14.560:
> > > remote_viewer_recent_add: assertion 'uri != NULL' failed
> > 
> > Right, this refactor is not straightforward on ovirt
> > codepath.  Thanks for testing it properly. I've sent a v3
> > without touching ovirt code now.
> 
> The warning is still present with your new patch, without it
> applied, no warnings are shown.

Ovirt is not that easy to poke. Couldn't reproduce the warning
because I'm not able to connect to running ovirt with Fedora 31.
Tried with master from libgovirt, no luck.

As mentioned in the commit log, passing a g_strdup() + g_free()
to a signal that can be triggered more than once is problematic
by itself so, suggestions welcome.

> > Cheers,
> > 
> > > 
> > > 
> > > >    static gchar *
> > > > @@ -675,14 +677,14 @@ read_all_stdin(gsize *len, GError **err)
> > > >    }
> > > >    static gboolean
> > > > -remote_viewer_initial_connect(RemoteViewer *self, const gchar *type, const gchar *guri,
> > > > +remote_viewer_initial_connect(RemoteViewer *self, const gchar *type,
> > > >                                  VirtViewerFile *vvfile, GError **error)
> > > >    {
> > > >        VirtViewerApp *app = VIRT_VIEWER_APP(self);
> > > >    #ifdef HAVE_OVIRT
> > > >        if (g_strcmp0(type, "ovirt") == 0) {
> > > > -        if (!create_ovirt_session(app, guri, error)) {
> > > > +        if (!create_ovirt_session(app, error)) {
> > > >                g_prefix_error(error, _("Couldn't open oVirt session: "));
> > > >                return FALSE;
> > > >            }
> > > > @@ -694,7 +696,7 @@ remote_viewer_initial_connect(RemoteViewer *self, const gchar *type, const gchar
> > > >        }
> > > >        g_signal_connect(virt_viewer_app_get_session(app), "session-connected",
> > > > -                     G_CALLBACK(remote_viewer_session_connected), g_strdup(guri));
> > > > +                     G_CALLBACK(remote_viewer_session_connected), app);
> > > >        virt_viewer_session_set_file(virt_viewer_app_get_session(app), vvfile);
> > > >    #ifdef HAVE_OVIRT
> > > > @@ -787,7 +789,7 @@ retry_dialog:
> > > >                                    _("Cannot determine the connection type from URI"));
> > > >                goto cleanup;
> > > >            }
> > > > -        if (!remote_viewer_initial_connect(self, type, guri, vvfile, &error))
> > > > +        if (!remote_viewer_initial_connect(self, type, vvfile, &error))
> > > >                goto cleanup;
> > > >        }
> > > > 
> > > 
> > > 
> > > -- 
> > > Eduardo de Barros Lima (Etrunko)
> > > Software Engineer - Red Hat
> > > etrunko at redhat.com
> 
> 
> -- 
> Eduardo de Barros Lima (Etrunko)
> Software Engineer - Red Hat
> etrunko at redhat.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20191001/24726bae/attachment.sig>