[virt-tools-list] [virt-viewer PATCH] Fix building with older spice-gtk

[Martin Kletzander]

On Thu, Mar 13, 2014 at 10:49:47AM +0100, Christophe Fergeau wrote:
> Hey,
>
> On Thu, Mar 13, 2014 at 10:16:08AM +0100, Martin Kletzander wrote:
> > Due to spice-gtk-0.23 missing SPICE_GTK_CHECK_VERSION macro, the
> > condition:
> >
> > causes the following error due to gcc not doing short-circuit evaluation:
>
> It should be doing short-circuit evaluation:
> http://gcc.gnu.org/onlinedocs/cpp/If.html
> Maybe what happens is that it starts by doing macro substitution, and then
> will evaluate the boolean expression, but it fails at doing so because the
> expanded expression is invalid.
>

I jumped to a conclusion without really investigating.  Your
explanation makes more sense now.  Sorry and thanks for clarifying.

> >
> > virt-viewer-session-spice.c: In function 'virt_viewer_session_spice_main_channel_event':
> > virt-viewer-session-spice.c:525:64: error: missing binary operator before token "("
> >  #if defined(SPICE_GTK_CHECK_VERSION) && SPICE_GTK_CHECK_VERSION(0, 23, 21)
> >                                                                 ^
> > Also one more warning is fixed in this patch:
> >
> > virt-viewer-session-spice.c:476:19: warning: unused variable 'error'
> > [-Wunused-variable] const GError *error;
> >                                   ^
> >
> > Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
> > ---
> >  src/virt-viewer-session-spice.c | 14 ++++++++++----
> >  1 file changed, 10 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/virt-viewer-session-spice.c b/src/virt-viewer-session-spice.c
> > index 1740ba3..fb63c9c 100644
> > --- a/src/virt-viewer-session-spice.c
> > +++ b/src/virt-viewer-session-spice.c
> > @@ -1,7 +1,7 @@
> >  /*
> >   * Virt Viewer: A virtual machine console viewer
> >   *
> > - * Copyright (C) 2007-2012 Red Hat, Inc.
> > + * Copyright (C) 2007-2012, 2014 Red Hat, Inc.
> >   * Copyright (C) 2009-2012 Daniel P. Berrange
> >   * Copyright (C) 2010 Marc-André Lureau
> >   *
> > @@ -473,7 +473,6 @@ virt_viewer_session_spice_main_channel_event(SpiceChannel *channel G_GNUC_UNUSED
> >                                               SpiceChannelEvent event,
> >                                               VirtViewerSession *session)
> >  {
> > -    const GError *error;
> >      VirtViewerSessionSpice *self = VIRT_VIEWER_SESSION_SPICE(session);
> >      gchar *password = NULL, *user = NULL;
> >      int ret;
> > @@ -522,8 +521,10 @@ virt_viewer_session_spice_main_channel_event(SpiceChannel *channel G_GNUC_UNUSED
> >          }
> >          break;
> >      case SPICE_CHANNEL_ERROR_CONNECT:
> > -#if defined(SPICE_GTK_CHECK_VERSION) && SPICE_GTK_CHECK_VERSION(0, 23, 21)
> > -        error = spice_channel_get_error(channel);
> > +#if defined(SPICE_GTK_CHECK_VERSION)
> > +# if SPICE_GTK_CHECK_VERSION(0, 23, 21)
>
> An alternative with less clutter here could be
> #ifndef SPICE_GTK_CHECK_VERSION
> #define SPICE_GTK_CHECK_VERSION(x, y, z) 0
> #endif
>
> at the top of the file, but ACK to the way you did it
>

That look *way* better, yes.

> > +    {
> > +        const GError *error = spice_channel_get_error(channel);
> >
> >          DEBUG_LOG("main channel: failed to connect %s", error ? error->message : "");
> >
> > @@ -545,6 +546,11 @@ virt_viewer_session_spice_main_channel_event(SpiceChannel *channel G_GNUC_UNUSED
> >          } else {
> >              g_signal_emit_by_name(session, "session-disconnected");
> >          }
> > +    }
>
>
>
> > +# else
> > +        DEBUG_LOG("main channel: failed to connect");
> > +        g_signal_emit_by_name(session, "session-disconnected");
> > +# endif
> >  #else
> >          DEBUG_LOG("main channel: failed to connect");
> >          g_signal_emit_by_name(session, "session-disconnected");
>
> This part of the patch is dubious, I'd expect this bit to be unchanged
>

Which it can be if we do the ifndef->define variant.

v2 coming up, thanks for he review.

Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20140313/31db7762/attachment.sig>