[Spice-devel] [PATCH spice-gtk 4/5] Narrow dbus-glib code, to fit other implementation easily

[Christophe Fergeau]

On Fri, Feb 07, 2014 at 01:22:34PM +0100, Marc-André Lureau wrote:
> On Fri, Feb 7, 2014 at 10:45 AM, Christophe Fergeau <cfergeau at redhat.com> wrote:
> > On Thu, Feb 06, 2014 at 01:55:54PM +0100, Marc-André Lureau wrote:
> >> From: Marc-André Lureau <marcandre.lureau at redhat.com>
> >>
> >> ---
> >>  gtk/desktop-integration.c | 55 +++++++++++++++++++++++++----------------------
> >>  1 file changed, 29 insertions(+), 26 deletions(-)
> >>
> >> diff --git a/gtk/desktop-integration.c b/gtk/desktop-integration.c
> >> index 06712d9..064994d 100644
> >> --- a/gtk/desktop-integration.c
> >> +++ b/gtk/desktop-integration.c
> >> @@ -22,7 +22,7 @@
> >>  #include "config.h"
> >>
> >>  #include <glib-object.h>
> >> -#ifdef USE_DBUS_GLIB
> >> +#if defined(USE_DBUS_GLIB)
> >>  #include <dbus/dbus-glib.h>
> >>  #endif
> >>
> >> @@ -41,12 +41,14 @@
> >>      (G_TYPE_INSTANCE_GET_PRIVATE ((obj), SPICE_TYPE_DESKTOP_INTEGRATION, SpiceDesktopIntegrationPrivate))
> >>
> >>  struct _SpiceDesktopIntegrationPrivate {
> >> -#ifdef USE_DBUS_GLIB
> >> +#if defined(USE_DBUS_GLIB)
> >>      DBusGProxy *gnome_session_proxy;
> >> -    guint gnome_automount_inhibit_cookie;
> >> +#elif defined(USE_GDBUS)
> >> +    GDBusProxy *gnome_session_proxy;
> >
> > This bit belongs in the next patch.
> >
> >>  #else
> >> -    int dummy;
> >> +    GObject *gnome_session_proxy; /* dummy */
> >>  #endif
> >> +    guint gnome_automount_inhibit_cookie;
> >>  };
> >>
> >>  G_DEFINE_TYPE(SpiceDesktopIntegration, spice_desktop_integration, G_TYPE_OBJECT);
> >> @@ -54,27 +56,28 @@ G_DEFINE_TYPE(SpiceDesktopIntegration, spice_desktop_integration, G_TYPE_OBJECT)
> >>  /* ------------------------------------------------------------------ */
> >>  /* Gnome specific code                                                */
> >>
> >> -#ifdef USE_DBUS_GLIB
> >> -
> >>  static void handle_dbus_call_error(const char *call, GError **_error)
> >>  {
> >>      GError *error = *_error;
> >>      const char *message = error->message;
> >>
> >> +#if defined(USE_DBUS_GLIB)
> >>      if (error->domain == DBUS_GERROR &&
> >>              error->code == DBUS_GERROR_REMOTE_EXCEPTION)
> >>          message = dbus_g_error_get_name(error);
> >> +#endif
> >>      g_warning("Error calling '%s': %s", call, message);
> >>      g_clear_error(_error);
> >>  }
> >>
> >>  static gboolean gnome_integration_init(SpiceDesktopIntegration *self)
> >>  {
> >> -    SpiceDesktopIntegrationPrivate *priv = self->priv;
> >> +    G_GNUC_UNUSED SpiceDesktopIntegrationPrivate *priv = self->priv;
> >>      GError *error = NULL;
> >> -    DBusGConnection *conn;
> >> +    gboolean success = TRUE;
> >>
> >> -    conn = dbus_g_bus_get(DBUS_BUS_SESSION, &error);
> >> +#if defined(USE_DBUS_GLIB)
> >> +    DBusGConnection *conn = dbus_g_bus_get(DBUS_BUS_SESSION, &error);
> >>      if (!conn)
> >>          goto end;
> >>
> >> @@ -87,14 +90,18 @@ static gboolean gnome_integration_init(SpiceDesktopIntegration *self)
> >>                                              "org.gnome.SessionManager",
> >>                                              &error);
> >>  end:
> >> +#else
> >> +    success = FALSE;
> >
> > Wondering if there should be some kind of compile-time or runtime warning
> > if this bit of code is reached?
> 
> there is run-time, at caller side.
> 
> But adding this to second patch could be helpful:
> 
> if test "x$have_dbus" = "xno"; then
>   AC_MSG_WARN([No D-Bus support, desktop function and USB may not work
> properly])
> fi

s/function/integration/ and s/USB/USB redirection/ ? but agreed, it makes
sense to squash this in patch #2

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20140207/302373b3/attachment.pgp>