Re: [PATCH 12/20] Replace RedCharDevice::on_free_self_token with a signal

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

 



> 
> From: Christophe Fergeau <cfergeau@xxxxxxxxxx>
> 
> It's more natural to do things this way with glib, and this allows to
> remove some coupling between Reds and RedCharDeviceVDIPort. Before this
> commit, the RedCharDeviceVDIPort has to provide a on_free_self_token()
> because Reds needs to know about it. With a signal, RedCharDeviceVDIPort
> does not have to do anything special, and Reds can just connect to the
> signal it's interested in.

This make no sense to me. Once you connect the signal you know the same
detail on the other class.
Personally I prefer the static binding that was in previous code.

Frediano

> ---
>  server/char-device.c | 47 ++++++++++++++++++++++++++++++++++++-----------
>  server/char-device.h |  4 ----
>  server/reds.c        |  7 +++++--
>  3 files changed, 41 insertions(+), 17 deletions(-)
> 
> diff --git a/server/char-device.c b/server/char-device.c
> index ebe7633..f3e16da 100644
> --- a/server/char-device.c
> +++ b/server/char-device.c
> @@ -94,6 +94,14 @@ enum {
>      PROP_OPAQUE
>  };
>  
> +enum {
> +   RED_CHAR_DEVICE_SELF_TOKEN_CONSUMED,
> +   RED_CHAR_DEVICE_SELF_TOKEN_RELEASED,
> +   RED_CHAR_DEVICE_LAST_SIGNAL
> +};
> +
> +static guint signals[RED_CHAR_DEVICE_LAST_SIGNAL];
> +
>  static void red_char_device_write_buffer_unref(RedCharDeviceWriteBuffer
>  *write_buf);
>  static void red_char_device_write_retry(void *opaque);
>  
> @@ -131,16 +139,6 @@ red_char_device_send_tokens_to_client(RedCharDevice
> *dev,
>  }
>  
>  static void
> -red_char_device_on_free_self_token(RedCharDevice *dev)
> -{
> -   RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
> -
> -   if (klass->on_free_self_token != NULL) {
> -       klass->on_free_self_token(dev->priv->opaque);
> -   }
> -}
> -
> -static void
>  red_char_device_remove_client(RedCharDevice *dev, RedClient *client)
>  {
>     RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
> @@ -624,6 +622,9 @@ static RedCharDeviceWriteBuffer
> *__red_char_device_write_buffer_get(
>          }
>      } else if (origin == WRITE_BUFFER_ORIGIN_SERVER) {
>          dev->priv->num_self_tokens--;
> +        g_signal_emit(G_OBJECT(dev),
> +                      signals[RED_CHAR_DEVICE_SELF_TOKEN_CONSUMED],
> +                      0);
>      }
>  
>      ret->token_price = migrated_data_tokens ? migrated_data_tokens : 1;
> @@ -711,7 +712,9 @@ void red_char_device_write_buffer_release(RedCharDevice
> *dev,
>          red_char_device_client_tokens_add(dev, dev_client, buf_token_price);
>      } else if (buf_origin == WRITE_BUFFER_ORIGIN_SERVER) {
>          dev->priv->num_self_tokens++;
> -        red_char_device_on_free_self_token(dev);
> +        g_signal_emit(G_OBJECT(dev),
> +                      signals[RED_CHAR_DEVICE_SELF_TOKEN_RELEASED],
> +                      0);
>      }
>  }
>  
> @@ -1215,6 +1218,28 @@ red_char_device_class_init(RedCharDeviceClass *klass)
>                                                           "User data to pass
>                                                           to callbacks",
>                                                        G_PARAM_STATIC_STRINGS
>                                                        |
>                                                        G_PARAM_READWRITE));
> +    signals[RED_CHAR_DEVICE_SELF_TOKEN_CONSUMED] =
> +       g_signal_new("self-token-consumed",
> +                    G_OBJECT_CLASS_TYPE(object_class),
> +                    G_SIGNAL_RUN_FIRST,
> +                    0, NULL, NULL,
> +                    g_cclosure_marshal_VOID__VOID,
> +                    G_TYPE_NONE,
> +                    0);
> +
> +    /* FIXME: find a better name for the signal? It replaces a
> +     * on_free_self_token vfunc whose description was:
> +     * « The cb is called when a server (self) message that was addressed to
> the device,
> +     *   has been completely written to it »
> +     */
> +    signals[RED_CHAR_DEVICE_SELF_TOKEN_RELEASED] =
> +       g_signal_new("self-token-released",
> +                    G_OBJECT_CLASS_TYPE(object_class),
> +                    G_SIGNAL_RUN_FIRST,
> +                    0, NULL, NULL,
> +                    g_cclosure_marshal_VOID__VOID,
> +                    G_TYPE_NONE,
> +                    0);
>  
>  }

Where these signals are destroyed ?

>  
> diff --git a/server/char-device.h b/server/char-device.h
> index f793c54..c18ce66 100644
> --- a/server/char-device.h
> +++ b/server/char-device.h
> @@ -67,10 +67,6 @@ struct RedCharDeviceClass
>       * device */
>      void (*send_tokens_to_client)(RedClient *client, uint32_t tokens, void
>      *opaque);
>  
> -    /* The cb is called when a server (self) message that was addressed to
> the device,
> -     * has been completely written to it */
> -    void (*on_free_self_token)(void *opaque);
> -
>      /* This cb is called if it is recommanded that a client will be removed
>       * due to slow flow or due to some other error.
>       * The called instance should disconnect the client, or at least the
>       corresponding channel */
> diff --git a/server/reds.c b/server/reds.c
> index 6e4fee4..7e5626a 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -876,7 +876,7 @@ static void vdi_port_send_tokens_to_client(RedClient
> *client, uint32_t tokens, v
>                                            tokens);
>  }
>  
> -static void vdi_port_on_free_self_token(void *opaque)
> +static void vdi_port_on_free_self_token(RedCharDevice *device, gpointer
> opaque)
>  {
>      RedsState *reds = opaque;
>  
> @@ -3377,6 +3377,10 @@ static int do_spice_init(RedsState *reds,
> SpiceCoreInterface *core_interface)
>      reds->listen_socket = -1;
>      reds->secure_listen_socket = -1;
>      reds->agent_dev = red_char_device_vdi_port_new(reds);
> +    g_signal_connect(G_OBJECT(reds->agent_dev),
> +                     "self-token-released",
> +                     (GCallback)vdi_port_on_free_self_token,
> +                     reds);
>      ring_init(&reds->clients);
>      reds->num_clients = 0;
>      reds->main_dispatcher = main_dispatcher_new(reds, reds->core);
> @@ -4322,7 +4326,6 @@
> red_char_device_vdi_port_class_init(RedCharDeviceVDIPortClass *klass)
>      char_dev_class->send_msg_to_client = vdi_port_send_msg_to_client;
>      char_dev_class->send_tokens_to_client = vdi_port_send_tokens_to_client;
>      char_dev_class->remove_client = vdi_port_remove_client;
> -    char_dev_class->on_free_self_token = vdi_port_on_free_self_token;
>  }
>  
>  static RedCharDeviceVDIPort *red_char_device_vdi_port_new(RedsState *reds)

Frediano
_______________________________________________
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]