On Mon, 2016-10-31 at 12:59 -0400, Frediano Ziglio wrote:
> >
> >
> > 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.
>
> From the patch looks like the opaque parameter can be removed.
Unfortunately, I don't think it can be removed at the moment, since
spicevmc uses a different object for its 'opaque' argument. I'll try to
see if I can refactor this further to get rid of the ugly 'opaque'
stuff.
>
> >
> > ---
> > server/char-device.c | 8 ++++----
> > server/char-device.h | 13 +++++++++----
> > server/reds.c | 17 ++++++++++++-----
> > server/smartcard.c | 29 ++++++++++++++++-------------
> > server/spicevmc.c | 12 ++++++++----
> > 5 files changed, 49 insertions(+), 30 deletions(-)
> >
> > diff --git a/server/char-device.c b/server/char-device.c
> > index 7775c07..8ed1a2a 100644
> > --- a/server/char-device.c
> > +++ b/server/char-device.c
> > @@ -99,7 +99,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,
> > dev->priv->opaque);
> > }
> >
> > static void
> > @@ -109,7 +109,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, dev->priv->opaque);
> > }
> >
> > static void
> > @@ -119,7 +119,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, dev->priv-
> > >opaque);
> > }
> >
> > static void
> > @@ -137,7 +137,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, dev->priv->opaque);
> > }
> >
> > static void
> > red_char_device_write_buffer_free(RedCharDeviceWriteBuffer *buf)
> > diff --git a/server/char-device.h b/server/char-device.h
> > index 44e9504..14cbe94 100644
> > --- a/server/char-device.h
> > +++ b/server/char-device.h
> > @@ -56,16 +56,21 @@ 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,
> > + RedPipeItem* (*read_one_msg_from_device)(RedCharDevice *self,
> > + SpiceCharDeviceInstan
> > ce *sin,
> > void *opaque);
> > /* after this call, the message is unreferenced */
> > - void (*send_msg_to_client)(RedPipeItem *msg,
> > + void (*send_msg_to_client)(RedCharDevice *self,
> > + RedPipeItem *msg,
> > RedClient *client,
> > void *opaque);
> >
> > /* 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,
> > + void *opaque);
> >
> > /* The cb is called when a server (self) message that was
> > addressed to
> > the device,
> > * has been completely written to it */
> > @@ -74,7 +79,7 @@ struct RedCharDeviceClass
> > /* 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,
> > void
> > *opaque);
> > };
> >
> > GType red_char_device_get_type(void) G_GNUC_CONST;
> > diff --git a/server/reds.c b/server/reds.c
> > index 9034197..0d9cef7 100644
> > --- a/server/reds.c
> > +++ b/server/reds.c
> > @@ -823,11 +823,12 @@ 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,
> > +static RedPipeItem
> > *vdi_port_read_one_msg_from_device(RedCharDevice *self,
> > +
> > SpiceCharDeviceInstance
> > *sin,
> > void
> > *opaque)
> > {
> > RedsState *reds;
> > - RedCharDeviceVDIPort *dev = RED_CHAR_DEVICE_VDIPORT(sin->st);
> > + RedCharDeviceVDIPort *dev = RED_CHAR_DEVICE_VDIPORT(self);
> > SpiceCharDeviceInterface *sif;
> > RedVDIReadBuf *dispatch_buf;
> > int n;
> > @@ -900,7 +901,8 @@ 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,
> > +static void vdi_port_send_msg_to_client(RedCharDevice *self,
> > + RedPipeItem *msg,
> > RedClient *client,
> > void *opaque)
> > {
> > @@ -914,7 +916,10 @@ 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,
> > + void *opaque)
> > {
> > main_channel_client_push_agent_tokens(red_client_get_main(clie
> > nt),
> > tokens);
> > @@ -930,7 +935,9 @@ 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,
> > + void *opaque)
> > {
> > red_channel_client_shutdown(RED_CHANNEL_CLIENT(red_client_get_
> > main(client)));
> > }
> > diff --git a/server/smartcard.c b/server/smartcard.c
> > index b37abb2..ed083d8 100644
> > --- a/server/smartcard.c
> > +++ b/server/smartcard.c
> > @@ -149,10 +149,11 @@ static void
> > smartcard_read_buf_prepare(RedCharDeviceSmartcard *dev,
> > VSCMsgHeader
> > }
> > }
> >
> > -static RedPipeItem
> > *smartcard_read_msg_from_device(SpiceCharDeviceInstance
> > *sin,
> > +static RedPipeItem *smartcard_read_msg_from_device(RedCharDevice
> > *self,
> > + SpiceCharDevice
> > Instance
> > *sin,
> > void *opaque)
> > {
> > - 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 +187,12 @@ static RedPipeItem
> > *smartcard_read_msg_from_device(SpiceCharDeviceInstance *sin,
> > return NULL;
> > }
> >
> > -static void smartcard_send_msg_to_client(RedPipeItem *msg,
> > +static void smartcard_send_msg_to_client(RedCharDevice *self,
> > + RedPipeItem *msg,
> > RedClient *client,
> > void *opaque)
> > {
> > - 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 +201,17 @@ 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,
> > + void *opaque)
> > {
> > spice_error("not implemented");
> > }
> >
> > -static void smartcard_remove_client(RedClient *client, void
> > *opaque)
> > +static void smartcard_remove_client(RedCharDevice *self, RedClient
> > *client,
> > void *opaque)
> > {
> > - 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 +252,7 @@ RedMsgItem
> > *smartcard_char_device_on_message_from_device(RedCharDeviceSmartcar
> > d
> >
> > 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 +277,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];
> > }
>
> These 2 hunks looks a bit OT.
>
> >
> > @@ -291,8 +296,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);
> > }
> >
>
> Shouldn't the property be removed?
>
> >
> > @@ -336,7 +339,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 +502,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 8b5acf5..99e5b6e 100644
> > --- a/server/spicevmc.c
> > +++ b/server/spicevmc.c
> > @@ -357,7 +357,8 @@ static RedVmcPipeItem*
> > try_compress_lz4(RedVmcChannel
> > *state, int n, RedVmcPipeI
> > }
> > #endif
> >
> > -static RedPipeItem
> > *spicevmc_chardev_read_msg_from_dev(SpiceCharDeviceInstance *sin,
> > +static RedPipeItem
> > *spicevmc_chardev_read_msg_from_dev(RedCharDevice *self,
> > +
> > SpiceCharDeviceInstance
> > *sin,
> > void
> > *opaque)
> > {
> > RedVmcChannel *state = opaque;
> > @@ -402,7 +403,8 @@ static RedPipeItem
> > *spicevmc_chardev_read_msg_from_dev(SpiceCharDeviceInstance *
> > }
> > }
> >
> > -static void spicevmc_chardev_send_msg_to_client(RedPipeItem *msg,
> > +static void spicevmc_chardev_send_msg_to_client(RedCharDevice
> > *self,
> > + RedPipeItem *msg,
> > RedClient *client,
> > void *opaque)
> > {
> > @@ -440,14 +442,16 @@ 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,
> > +static void spicevmc_char_dev_send_tokens_to_client(RedCharDevice
> > *self,
> > + RedClient
> > *client,
> > uint32_t
> > tokens,
> > void *opaque)
> > {
> > 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,
> > void *opaque)
> > {
> > RedVmcChannel *state = opaque;
> >
>
> Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel