Re: [PATCH 15/20] Use weak gobject ref instead of reds_on_char_device_state_destroy

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 2016-04-14 at 16:50 -0500, Jonathon Jongsma wrote:
> From: Christophe Fergeau <cfergeau@xxxxxxxxxx>
> 
> RedCharDevice implementation had to callback into reds.c in order to let
> it know a char device was being destroyed. Now that RedCharDevice is a
> gobject, a weak reference can be used instead allowing to remove that
> coupling.
> ---
>  server/char-device.c |  2 --
>  server/reds.c        | 18 +++++++++++-------
>  server/reds.h        |  1 -
>  3 files changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/server/char-device.c b/server/char-device.c
> index 6b9596e..b4f25c7 100644
> --- a/server/char-device.c
> +++ b/server/char-device.c
> @@ -1121,8 +1121,6 @@ red_char_device_finalize(GObject *object)
>  {
>      RedCharDevice *self = RED_CHAR_DEVICE(object);
>  
> -    /* FIXME: replace with g_object_weak_ref () */
> -    reds_on_char_device_state_destroy(self->priv->reds, self);
>      if (self->priv->write_to_dev_timer) {
>          reds_core_timer_remove(self->priv->reds, self->priv
> ->write_to_dev_timer);
>          self->priv->write_to_dev_timer = NULL;
> diff --git a/server/reds.c b/server/reds.c
> index 3c95864..3a23650 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -252,7 +252,6 @@ static void
> reds_mig_target_client_free(RedsMigTargetClient *mig_client);
>  static void reds_mig_cleanup_wait_disconnect(RedsState *reds);
>  static void reds_mig_remove_wait_disconnect_client(RedsState *reds, RedClient
> *client);
>  static void reds_add_char_device(RedsState *reds, RedCharDevice *dev);
> -static void reds_remove_char_device(RedsState *reds, RedCharDevice *dev);
>  static void reds_send_mm_time(RedsState *reds);
>  static void reds_on_ic_change(RedsState *reds);
>  static void reds_on_sv_change(RedsState *reds);
> @@ -3142,15 +3141,13 @@ static void reds_add_char_device(RedsState *reds,
> RedCharDevice *dev)
>      reds->char_devices = g_list_append(reds->char_devices, dev);
>  }
>  
> -static void reds_remove_char_device(RedsState *reds, RedCharDevice *dev)
> +static void reds_on_char_device_state_destroy(RedsState *reds,
> +                                              RedCharDevice *dev)
>  {
> +    g_return_if_fail(reds != NULL);
>      g_warn_if_fail(g_list_find(reds->char_devices, dev) != NULL);
> -    reds->char_devices = g_list_remove(reds->char_devices, dev);
> -}
>  
> -void reds_on_char_device_state_destroy(RedsState *reds, RedCharDevice *dev)
> -{
> -    reds_remove_char_device(reds, dev);
> +    reds->char_devices = g_list_remove(reds->char_devices, dev);
>  }
>  
>  static int spice_server_char_device_add_interface(SpiceServer *reds,
> @@ -3187,7 +3184,14 @@ static int
> spice_server_char_device_add_interface(SpiceServer *reds,
>      }
>  
>      if (dev_state) {
> +        RedsState *reds;
> +
>          spice_assert(char_device->st);
> +
> +        g_object_get(G_OBJECT(dev_state), "spice-server", &reds, NULL);
> +        g_object_weak_ref(G_OBJECT(dev_state),
> +                          (GWeakNotify)reds_on_char_device_state_destroy,
> +                          reds);

This works, but I'd personally prefer to move the weak ref down a few lines to a
point after it has been added to the GList.

Otherwise looks good

Acked-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>



>          /* setting the char_device state to "started" for backward
> compatibily with
>           * qemu releases that don't call spice api for start/stop (not
> implemented yet) */
>          if (reds->vm_running) {
> diff --git a/server/reds.h b/server/reds.h
> index 2cfd451..5b33432 100644
> --- a/server/reds.h
> +++ b/server/reds.h
> @@ -100,7 +100,6 @@ int reds_on_migrate_dst_set_seamless(RedsState *reds,
> MainChannelClient *mcc, ui
>  void reds_on_client_semi_seamless_migrate_complete(RedsState *reds, RedClient
> *client);
>  void reds_on_client_seamless_migrate_complete(RedsState *reds, RedClient
> *client);
>  void reds_on_main_channel_migrate(RedsState *reds, MainChannelClient *mcc);
> -void reds_on_char_device_state_destroy(RedsState *reds, RedCharDevice *dev);
>  
>  void reds_set_client_mm_time_latency(RedsState *reds, RedClient *client,
> uint32_t latency);
>  uint32_t reds_get_streaming_video(const RedsState *reds);
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]