[virt-tools-list] [PATCH virt-viewer] spice: avoid crash if connection failed without error

[Marc-André Lureau]

----- Original Message -----
> On Mon, Jul 21, 2014 at 10:59 AM, Marc-André Lureau
> <marcandre.lureau at gmail.com> wrote:
> > spice_channel_get_error() is not guarantee to return a GError.
> > ---
> >  src/virt-viewer-session-spice.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/virt-viewer-session-spice.c
> > b/src/virt-viewer-session-spice.c
> > index 2d4e67d..8b90d66 100644
> > --- a/src/virt-viewer-session-spice.c
> > +++ b/src/virt-viewer-session-spice.c
> > @@ -547,7 +547,7 @@
> > virt_viewer_session_spice_main_channel_event(SpiceChannel *channel
> > G_GNUC_UNUSED
> >                  spice_session_connect(self->priv->session);
> >              }
> >          } else {
> > -            g_signal_emit_by_name(session, "session-disconnected",
> > error->message);
> > +            g_signal_emit_by_name(session, "session-disconnected",
> > error ? error->message : NULL);
> >          }
> >      }
> >  #else
> > --
> > 1.9.3
> > 
> > _______________________________________________
> > virt-tools-list mailing list
> > virt-tools-list at redhat.com
> > https://www.redhat.com/mailman/listinfo/virt-tools-list
> 
> 
> ACK.
> 
> In some ways, I think it would be nicer if the "session-disconnected"
> signal actually passed the whole GError along instead of just the
> message. But I'm not sure it's worth changing now. Is there a related
> bug for this issue?  If so, can you add a reference to the commit
> message?

There is no related bug. I noticed the crash by introducing an invalid spice URL.

The spice_channel_get_error() was introduced without guarantee when it will 
return an error, you must always check if it is non-NULL. (Historically, errors
have been returned via SpiceChannelEvent, but an enum isn't flexible enough
and can't map all possible underlying GErrors details..)