[Spice-devel] [PATCH 7/9] char-device: add 'self' param to vfuncs

Frediano Ziglio fziglio at redhat.com
Mon Oct 31 16:59:42 UTC 2016


> 
> 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.

> ---
>  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,
> +                                             SpiceCharDeviceInstance *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(client),
>                                            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,
> +                                                   SpiceCharDeviceInstance
> *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(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 +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


More information about the Spice-devel mailing list