[Spice-devel] [PATCH spice-gtk 08/15] gtk: do not require glib session private fields

[Jonathon Jongsma]

On Tue, 2014-11-25 at 14:19 +0100, Marc-André Lureau wrote:
> Use GObject object association for session helpers.

So, you just added internal accessor functions for a bunch of other
session member variables (_get_webdav_magic(), etc.). But for these, you
used the GObject data functions. Why are these variables treated
differently? By using g_object_get|set_data(), you lose some type
safety, and make things harder to debug since you can't look directly at
struct members, etc. I'd really rather just add setters/getters for
these variables.


> ---
>  gtk/desktop-integration.c | 7 ++-----
>  gtk/spice-gtk-session.c   | 4 ++--
>  gtk/spice-session-priv.h  | 2 --
>  gtk/spice-session.c       | 2 --
>  4 files changed, 4 insertions(+), 11 deletions(-)
> 
> diff --git a/gtk/desktop-integration.c b/gtk/desktop-integration.c
> index 145fa6a..c366fd0 100644
> --- a/gtk/desktop-integration.c
> +++ b/gtk/desktop-integration.c
> @@ -187,9 +187,6 @@ static void spice_desktop_integration_class_init(SpiceDesktopIntegrationClass *k
>      g_type_class_add_private(klass, sizeof(SpiceDesktopIntegrationPrivate));
>  }
>  
> -/* ------------------------------------------------------------------ */
> -/* public methods                                                     */
> -
>  SpiceDesktopIntegration *spice_desktop_integration_get(SpiceSession *session)
>  {
>      SpiceDesktopIntegration *self;
> @@ -198,10 +195,10 @@ SpiceDesktopIntegration *spice_desktop_integration_get(SpiceSession *session)
>      g_return_val_if_fail(session != NULL, NULL);
>  
>      g_static_mutex_lock(&mutex);
> -    self = session->priv->desktop_integration;
> +    self = g_object_get_data(G_OBJECT(session), "spice-desktop");
>      if (self == NULL) {
>          self = g_object_new(SPICE_TYPE_DESKTOP_INTEGRATION, NULL);
> -        session->priv->desktop_integration = self;
> +        g_object_set_data_full(G_OBJECT(session), "spice-desktop", self, g_object_unref);
>      }
>      g_static_mutex_unlock(&mutex);
>  
> diff --git a/gtk/spice-gtk-session.c b/gtk/spice-gtk-session.c
> index 701950d..de01358 100644
> --- a/gtk/spice-gtk-session.c
> +++ b/gtk/spice-gtk-session.c
> @@ -1119,10 +1119,10 @@ SpiceGtkSession *spice_gtk_session_get(SpiceSession *session)
>      static GStaticMutex mutex = G_STATIC_MUTEX_INIT;
>  
>      g_static_mutex_lock(&mutex);
> -    self = session->priv->gtk_session;
> +    self = g_object_get_data(G_OBJECT(session), "spice-gtk-session");
>      if (self == NULL) {
>          self = g_object_new(SPICE_TYPE_GTK_SESSION, "session", session, NULL);
> -        session->priv->gtk_session = self;
> +        g_object_set_data_full(G_OBJECT(session), "spice-gtk-session", self, g_object_unref);
>      }
>      g_static_mutex_unlock(&mutex);
>  
> diff --git a/gtk/spice-session-priv.h b/gtk/spice-session-priv.h
> index 5136c2d..9acfcfe 100644
> --- a/gtk/spice-session-priv.h
> +++ b/gtk/spice-session-priv.h
> @@ -114,8 +114,6 @@ struct _SpiceSessionPrivate {
>  
>      /* associated objects */
>      SpiceAudio        *audio_manager;
> -    SpiceDesktopIntegration *desktop_integration;
> -    SpiceGtkSession   *gtk_session;
>      SpiceUsbDeviceManager *usb_manager;
>      SpicePlaybackChannel *playback_channel;
>      PhodavServer      *webdav;
> diff --git a/gtk/spice-session.c b/gtk/spice-session.c
> index 2850d02..9e9cb5d 100644
> --- a/gtk/spice-session.c
> +++ b/gtk/spice-session.c
> @@ -186,8 +186,6 @@ spice_session_dispose(GObject *gobject)
>      g_warn_if_fail(s->after_main_init == 0);
>  
>      g_clear_object(&s->audio_manager);
> -    g_clear_object(&s->desktop_integration);
> -    g_clear_object(&s->gtk_session);
>      g_clear_object(&s->usb_manager);
>      g_clear_object(&s->proxy);
>      g_clear_object(&s->webdav);