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

[Christophe Fergeau]

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.

> 
> 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

> +    {
> +        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




> -- 
> 1.9.0
> 
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list at redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20140313/3a3290e3/attachment.sig>