[Spice-devel] [PATCH 12/20] Replace RedCharDevice::on_free_self_token with a signal
Frediano Ziglio
fziglio at redhat.com
Fri Apr 15 10:18:47 UTC 2016
>
> From: Christophe Fergeau <cfergeau at redhat.com>
>
> 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
More information about the Spice-devel
mailing list