[Spice-devel] [PATCH spice-server] char-device: add 'self' param to vfuncs
Frediano Ziglio
fziglio at redhat.com
Mon Nov 14 09:35:24 UTC 2016
>
> From: Jonathon Jongsma <jjongsma at redhat.com>
>
> Add a 'self' parameter to all of the char device virtual functions so
> that we don't have to play games with the 'opaque' pointer.
I would ack this.
Just as a did minor changes can somebody confirm?
Frediano
> ---
> server/char-device.c | 31 +++++--------------------------
> server/char-device.h | 20 ++++++++++----------
> server/reds.c | 24 +++++++++++++-----------
> server/smartcard-channel-client.c | 2 +-
> server/smartcard.c | 32 ++++++++++++++++----------------
> server/spicevmc.c | 37 ++++++++++++++++++++++---------------
> 6 files changed, 67 insertions(+), 79 deletions(-)
>
> Change since last version:
> - rebased and updated.
>
> Don't remember why this was not merged but the rebase was
> not automatic but I think patch is worth and still valid.
> Did some test.
>
> diff --git a/server/char-device.c b/server/char-device.c
> index 318bf3c..3b70066 100644
> --- a/server/char-device.c
> +++ b/server/char-device.c
> @@ -66,7 +66,6 @@ struct RedCharDevicePrivate {
> int during_read_from_device;
> int during_write_to_device;
>
> - void *opaque;
> SpiceServer *reds;
> };
>
> @@ -88,7 +87,6 @@ enum {
> PROP_SPICE_SERVER,
> PROP_CLIENT_TOKENS_INTERVAL,
> PROP_SELF_TOKENS,
> - PROP_OPAQUE
> };
>
> static void red_char_device_write_buffer_unref(RedCharDeviceWriteBuffer
> *write_buf);
> @@ -99,7 +97,7 @@ red_char_device_read_one_msg_from_device(RedCharDevice
> *dev)
> {
> RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
>
> - return klass->read_one_msg_from_device(dev->priv->sin,
> dev->priv->opaque);
> + return klass->read_one_msg_from_device(dev, dev->priv->sin);
> }
>
> static void
> @@ -109,7 +107,7 @@ red_char_device_send_msg_to_client(RedCharDevice *dev,
> {
> RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
>
> - klass->send_msg_to_client(msg, client, dev->priv->opaque);
> + klass->send_msg_to_client(dev, msg, client);
> }
>
> static void
> @@ -119,7 +117,7 @@ red_char_device_send_tokens_to_client(RedCharDevice *dev,
> {
> RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
>
> - klass->send_tokens_to_client(client, tokens, dev->priv->opaque);
> + klass->send_tokens_to_client(dev, client, tokens);
> }
>
> static void
> @@ -128,7 +126,7 @@ 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);
> + klass->on_free_self_token(dev);
> }
> }
>
> @@ -137,7 +135,7 @@ red_char_device_remove_client(RedCharDevice *dev,
> RedClient *client)
> {
> RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
>
> - klass->remove_client(client, dev->priv->opaque);
> + klass->remove_client(dev, client);
> }
>
> static void red_char_device_write_buffer_free(RedCharDeviceWriteBuffer *buf)
> @@ -686,11 +684,6 @@ void red_char_device_reset_dev_instance(RedCharDevice
> *dev,
> g_object_notify(G_OBJECT(dev), "sin");
> }
>
> -void *red_char_device_opaque_get(RedCharDevice *dev)
> -{
> - return dev->priv->opaque;
> -}
> -
> void red_char_device_destroy(RedCharDevice *char_dev)
> {
> g_return_if_fail(RED_IS_CHAR_DEVICE(char_dev));
> @@ -1041,9 +1034,6 @@ red_char_device_get_property(GObject *object,
> case PROP_SELF_TOKENS:
> g_value_set_uint64(value, self->priv->num_self_tokens);
> break;
> - case PROP_OPAQUE:
> - g_value_set_pointer(value, self->priv->opaque);
> - break;
> default:
> G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id, pspec);
> }
> @@ -1071,9 +1061,6 @@ red_char_device_set_property(GObject *object,
> case PROP_SELF_TOKENS:
> self->priv->num_self_tokens = g_value_get_uint64(value);
> break;
> - case PROP_OPAQUE:
> - self->priv->opaque = g_value_get_pointer(value);
> - break;
> default:
> G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id, pspec);
> }
> @@ -1156,14 +1143,6 @@ red_char_device_class_init(RedCharDeviceClass *klass)
> 0, G_MAXUINT64, 0,
> G_PARAM_STATIC_STRINGS
> |
> G_PARAM_READWRITE));
> - g_object_class_install_property(object_class,
> - PROP_OPAQUE,
> - g_param_spec_pointer("opaque",
> - "opaque",
> - "User data to pass
> to callbacks",
> - G_PARAM_STATIC_STRINGS
> |
> - G_PARAM_READWRITE));
> -
> }
>
> static void
> diff --git a/server/char-device.h b/server/char-device.h
> index 44e9504..3b87023 100644
> --- a/server/char-device.h
> +++ b/server/char-device.h
> @@ -56,25 +56,27 @@ struct RedCharDeviceClass
>
> /* reads from the device till reaching a msg that should be sent to the
> client,
> * or till the reading fails */
> - RedPipeItem* (*read_one_msg_from_device)(SpiceCharDeviceInstance *sin,
> - void *opaque);
> + RedPipeItem* (*read_one_msg_from_device)(RedCharDevice *self,
> + SpiceCharDeviceInstance *sin);
> /* after this call, the message is unreferenced */
> - void (*send_msg_to_client)(RedPipeItem *msg,
> - RedClient *client,
> - void *opaque);
> + void (*send_msg_to_client)(RedCharDevice *self,
> + RedPipeItem *msg,
> + RedClient *client);
>
> /* The cb is called when a predefined number of write buffers were
> consumed by the
> * device */
> - void (*send_tokens_to_client)(RedClient *client, uint32_t tokens, void
> *opaque);
> + void (*send_tokens_to_client)(RedCharDevice *self,
> + RedClient *client,
> + uint32_t tokens);
>
> /* 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);
> + void (*on_free_self_token)(RedCharDevice *self);
>
> /* This cb is called if it is recommended to remove the client
> * due to slow flow or due to some other error.
> * The called instance should disconnect the client, or at least the
> corresponding channel */
> - void (*remove_client)(RedClient *client, void *opaque);
> + void (*remove_client)(RedCharDevice *self, RedClient *client);
> };
>
> GType red_char_device_get_type(void) G_GNUC_CONST;
> @@ -159,8 +161,6 @@ void red_char_device_reset_dev_instance(RedCharDevice
> *dev,
> SpiceCharDeviceInstance *sin);
> void red_char_device_destroy(RedCharDevice *dev);
>
> -void *red_char_device_opaque_get(RedCharDevice *dev);
> -
> /* only one client is supported */
> void red_char_device_migrate_data_marshall(RedCharDevice *dev,
> SpiceMarshaller *m);
> diff --git a/server/reds.c b/server/reds.c
> index 30b9165..5daf9bd 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -817,11 +817,11 @@ static void vdi_port_read_buf_free(RedPipeItem *base)
>
> /* reads from the device till completes reading a message that is addressed
> to the client,
> * or otherwise, when reading from the device fails */
> -static RedPipeItem
> *vdi_port_read_one_msg_from_device(SpiceCharDeviceInstance *sin,
> - void *opaque)
> +static RedPipeItem *vdi_port_read_one_msg_from_device(RedCharDevice *self,
> +
> SpiceCharDeviceInstance
> *sin)
> {
> RedsState *reds;
> - RedCharDeviceVDIPort *dev = RED_CHAR_DEVICE_VDIPORT(sin->st);
> + RedCharDeviceVDIPort *dev = RED_CHAR_DEVICE_VDIPORT(self);
> SpiceCharDeviceInterface *sif;
> RedVDIReadBuf *dispatch_buf;
> int n;
> @@ -894,9 +894,9 @@ static RedPipeItem
> *vdi_port_read_one_msg_from_device(SpiceCharDeviceInstance *s
> }
>
> /* after calling this, we unref the message, and the ref is in the instance
> side */
> -static void vdi_port_send_msg_to_client(RedPipeItem *msg,
> - RedClient *client,
> - void *opaque)
> +static void vdi_port_send_msg_to_client(RedCharDevice *self,
> + RedPipeItem *msg,
> + RedClient *client)
> {
> RedVDIReadBuf *agent_data_buf = (RedVDIReadBuf *)msg;
>
> @@ -908,15 +908,17 @@ static void vdi_port_send_msg_to_client(RedPipeItem
> *msg,
> agent_data_buf);
> }
>
> -static void vdi_port_send_tokens_to_client(RedClient *client, uint32_t
> tokens, void *opaque)
> +static void vdi_port_send_tokens_to_client(RedCharDevice *self,
> + RedClient *client,
> + uint32_t tokens)
> {
> main_channel_client_push_agent_tokens(red_client_get_main(client),
> tokens);
> }
>
> -static void vdi_port_on_free_self_token(void *opaque)
> +static void vdi_port_on_free_self_token(RedCharDevice *self)
> {
> - RedsState *reds = opaque;
> + RedsState *reds = red_char_device_get_server(self);
>
> if (reds->inputs_channel && reds->pending_mouse_event) {
> spice_debug("pending mouse event");
> @@ -924,7 +926,8 @@ static void vdi_port_on_free_self_token(void *opaque)
> }
> }
>
> -static void vdi_port_remove_client(RedClient *client, void *opaque)
> +static void vdi_port_remove_client(RedCharDevice *self,
> + RedClient *client)
> {
> red_channel_client_shutdown(RED_CHANNEL_CLIENT(red_client_get_main(client)));
> }
> @@ -4514,6 +4517,5 @@ static RedCharDeviceVDIPort
> *red_char_device_vdi_port_new(RedsState *reds)
> "spice-server", reds,
> "client-tokens-interval",
> (guint64)REDS_TOKENS_TO_SEND,
> "self-tokens",
> (guint64)REDS_NUM_INTERNAL_AGENT_MESSAGES,
> - "opaque", reds,
> NULL);
> }
> diff --git a/server/smartcard-channel-client.c
> b/server/smartcard-channel-client.c
> index 30b2249..d00a6b2 100644
> --- a/server/smartcard-channel-client.c
> +++ b/server/smartcard-channel-client.c
> @@ -273,7 +273,7 @@ static void
> smartcard_channel_client_remove_reader(SmartCardChannelClient *scc,
> return;
> }
>
> - dev = red_char_device_opaque_get(char_device->st);
> + dev = RED_CHAR_DEVICE_SMARTCARD(char_device->st);
> spice_assert(scc->priv->smartcard == dev);
> if (!smartcard_char_device_notify_reader_remove(dev)) {
> smartcard_channel_client_push_error(RED_CHANNEL_CLIENT(scc),
> diff --git a/server/smartcard.c b/server/smartcard.c
> index 53919c0..5b18abe 100644
> --- a/server/smartcard.c
> +++ b/server/smartcard.c
> @@ -149,10 +149,10 @@ static void
> smartcard_read_buf_prepare(RedCharDeviceSmartcard *dev, VSCMsgHeader
> }
> }
>
> -static RedPipeItem *smartcard_read_msg_from_device(SpiceCharDeviceInstance
> *sin,
> - void *opaque)
> +static RedPipeItem *smartcard_read_msg_from_device(RedCharDevice *self,
> + SpiceCharDeviceInstance
> *sin)
> {
> - RedCharDeviceSmartcard *dev = opaque;
> + RedCharDeviceSmartcard *dev = RED_CHAR_DEVICE_SMARTCARD(self);
> SpiceCharDeviceInterface *sif = spice_char_device_get_interface(sin);
> VSCMsgHeader *vheader = (VSCMsgHeader*)dev->priv->buf;
> int n;
> @@ -186,11 +186,11 @@ static RedPipeItem
> *smartcard_read_msg_from_device(SpiceCharDeviceInstance *sin,
> return NULL;
> }
>
> -static void smartcard_send_msg_to_client(RedPipeItem *msg,
> - RedClient *client,
> - void *opaque)
> +static void smartcard_send_msg_to_client(RedCharDevice *self,
> + RedPipeItem *msg,
> + RedClient *client)
> {
> - RedCharDeviceSmartcard *dev = opaque;
> + RedCharDeviceSmartcard *dev = RED_CHAR_DEVICE_SMARTCARD(self);
> RedChannelClient *rcc = RED_CHANNEL_CLIENT(dev->priv->scc);
>
> spice_assert(dev->priv->scc &&
> @@ -199,14 +199,16 @@ static void smartcard_send_msg_to_client(RedPipeItem
> *msg,
> smartcard_channel_client_pipe_add_push(rcc, msg);
> }
>
> -static void smartcard_send_tokens_to_client(RedClient *client, uint32_t
> tokens, void *opaque)
> +static void smartcard_send_tokens_to_client(RedCharDevice *self,
> + RedClient *client,
> + uint32_t tokens)
> {
> spice_error("not implemented");
> }
>
> -static void smartcard_remove_client(RedClient *client, void *opaque)
> +static void smartcard_remove_client(RedCharDevice *self, RedClient *client)
> {
> - RedCharDeviceSmartcard *dev = opaque;
> + RedCharDeviceSmartcard *dev = RED_CHAR_DEVICE_SMARTCARD(self);
> RedChannelClient *rcc = RED_CHANNEL_CLIENT(dev->priv->scc);
>
> spice_printerr("smartcard dev %p, client %p", dev, client);
> @@ -247,7 +249,7 @@ RedMsgItem
> *smartcard_char_device_on_message_from_device(RedCharDeviceSmartcard
>
> static int smartcard_char_device_add_to_readers(RedsState *reds,
> SpiceCharDeviceInstance *char_device)
> {
> - RedCharDeviceSmartcard *dev =
> red_char_device_opaque_get(char_device->st);
> + RedCharDeviceSmartcard *dev =
> RED_CHAR_DEVICE_SMARTCARD(char_device->st);
>
> if (g_smartcard_readers.num >= SMARTCARD_MAX_READERS) {
> return -1;
> @@ -272,7 +274,7 @@ SpiceCharDeviceInstance
> *smartcard_readers_get_unattached(void)
> RedCharDeviceSmartcard* dev;
>
> for (i = 0; i < g_smartcard_readers.num; ++i) {
> - dev = red_char_device_opaque_get(g_smartcard_readers.sin[i]->st);
> + dev = RED_CHAR_DEVICE_SMARTCARD(g_smartcard_readers.sin[i]->st);
> if (!dev->priv->scc) {
> return g_smartcard_readers.sin[i];
> }
> @@ -291,8 +293,6 @@ static RedCharDeviceSmartcard
> *smartcard_device_new(RedsState *reds, SpiceCharDe
> "self-tokens", ~0ULL,
> NULL);
>
> - g_object_set(char_dev, "opaque", char_dev, NULL);
> -
> return RED_CHAR_DEVICE_SMARTCARD(char_dev);
> }
>
> @@ -336,7 +336,7 @@ void
> smartcard_char_device_notify_reader_add(RedCharDeviceSmartcard *dev)
> void smartcard_char_device_attach_client(SpiceCharDeviceInstance
> *char_device,
> SmartCardChannelClient *scc)
> {
> - RedCharDeviceSmartcard *dev =
> red_char_device_opaque_get(char_device->st);
> + RedCharDeviceSmartcard *dev =
> RED_CHAR_DEVICE_SMARTCARD(char_device->st);
> int client_added;
>
> spice_assert(!smartcard_channel_client_get_char_device(scc) &&
> !dev->priv->scc);
> @@ -499,7 +499,7 @@ void
> smartcard_channel_write_to_reader(RedCharDeviceWriteBuffer *write_buf)
>
> spice_assert(vheader->reader_id <= g_smartcard_readers.num);
> sin = g_smartcard_readers.sin[vheader->reader_id];
> - dev = (RedCharDeviceSmartcard *)red_char_device_opaque_get(sin->st);
> + dev = RED_CHAR_DEVICE_SMARTCARD(sin->st);
> spice_assert(!dev->priv->scc ||
> dev ==
> smartcard_channel_client_get_device(dev->priv->scc));
> /* protocol requires messages to be in network endianess */
> diff --git a/server/spicevmc.c b/server/spicevmc.c
> index 532598d..c19e996 100644
> --- a/server/spicevmc.c
> +++ b/server/spicevmc.c
> @@ -86,6 +86,9 @@ struct RedCharDeviceSpiceVmcClass
> RedCharDeviceClass parent_class;
> };
>
> +typedef struct RedVmcChannel RedVmcChannel;
> +typedef struct RedVmcChannelClass RedVmcChannelClass;
> +
> static GType red_char_device_spicevmc_get_type(void) G_GNUC_CONST;
> static RedCharDevice *red_char_device_spicevmc_new(SpiceCharDeviceInstance
> *sin,
> RedsState *reds,
> @@ -318,10 +321,11 @@ static RedVmcPipeItem* try_compress_lz4(RedVmcChannel
> *channel, int n, RedVmcPip
> }
> #endif
>
> -static RedPipeItem
> *spicevmc_chardev_read_msg_from_dev(SpiceCharDeviceInstance *sin,
> - void *opaque)
> +static RedPipeItem *spicevmc_chardev_read_msg_from_dev(RedCharDevice *self,
> +
> SpiceCharDeviceInstance
> *sin)
> {
> - RedVmcChannel *channel = opaque;
> + RedCharDeviceSpiceVmc *vmc = RED_CHAR_DEVICE_SPICEVMC(self);
> + RedVmcChannel *channel = RED_VMC_CHANNEL(vmc->channel);
> SpiceCharDeviceInterface *sif;
> RedVmcPipeItem *msg_item;
> int n;
> @@ -363,11 +367,12 @@ static RedPipeItem
> *spicevmc_chardev_read_msg_from_dev(SpiceCharDeviceInstance *
> }
> }
>
> -static void spicevmc_chardev_send_msg_to_client(RedPipeItem *msg,
> - RedClient *client,
> - void *opaque)
> +static void spicevmc_chardev_send_msg_to_client(RedCharDevice *self,
> + RedPipeItem *msg,
> + RedClient *client)
> {
> - RedVmcChannel *channel = opaque;
> + RedCharDeviceSpiceVmc *vmc = RED_CHAR_DEVICE_SPICEVMC(self);
> + RedVmcChannel *channel = RED_VMC_CHANNEL(vmc->channel);
>
> spice_assert(red_channel_client_get_client(channel->rcc) == client);
> red_pipe_item_ref(msg);
> @@ -395,16 +400,18 @@ static void spicevmc_port_send_event(RedChannelClient
> *rcc, uint8_t event)
> red_channel_client_pipe_add_push(rcc, &item->base);
> }
>
> -static void spicevmc_char_dev_send_tokens_to_client(RedClient *client,
> - uint32_t tokens,
> - void *opaque)
> +static void spicevmc_char_dev_send_tokens_to_client(RedCharDevice *self,
> + RedClient *client,
> + uint32_t tokens)
> {
> spice_printerr("Not implemented!");
> }
>
> -static void spicevmc_char_dev_remove_client(RedClient *client, void *opaque)
> +static void spicevmc_char_dev_remove_client(RedCharDevice *self,
> + RedClient *client)
> {
> - RedVmcChannel *channel = opaque;
> + RedCharDeviceSpiceVmc *vmc = RED_CHAR_DEVICE_SPICEVMC(self);
> + RedVmcChannel *channel = RED_VMC_CHANNEL(vmc->channel);
>
> spice_printerr("vmc channel %p, client %p", channel, client);
> spice_assert(channel->rcc &&
> @@ -831,14 +838,15 @@ void spicevmc_device_disconnect(RedsState *reds,
> SpiceCharDeviceInstance *sin)
> SPICE_GNUC_VISIBLE void spice_server_port_event(SpiceCharDeviceInstance
> *sin, uint8_t event)
> {
> RedVmcChannel *channel;
> + RedCharDeviceSpiceVmc *device = RED_CHAR_DEVICE_SPICEVMC(sin->st);
>
> if (sin->st == NULL) {
> spice_warning("no SpiceCharDeviceState attached to instance %p",
> sin);
> return;
> }
>
> - /* FIXME */
> - channel = (RedVmcChannel
> *)red_char_device_opaque_get((RedCharDevice*)sin->st);
> + channel = RED_VMC_CHANNEL(device->channel);
> +
> if (event == SPICE_PORT_EVENT_OPENED) {
> channel->port_opened = TRUE;
> } else if (event == SPICE_PORT_EVENT_CLOSED) {
> @@ -941,6 +949,5 @@ red_char_device_spicevmc_new(SpiceCharDeviceInstance
> *sin,
> "client-tokens-interval", 0ULL,
> "self-tokens", ~0ULL,
> "channel", channel,
> - "opaque", channel,
> NULL);
> }
More information about the Spice-devel
mailing list