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@xxxxxxxxxx> > > --- > > 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
Attachment:
signature.asc
Description: Digital signature
_______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
