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