[Spice-devel] [PATCH spice-server v2 1/2] char-device: Don't use RedClient API

Victor Toso victortoso at redhat.com
Tue Sep 17 09:46:36 UTC 2019


On Sun, Jun 02, 2019 at 08:32:25PM +0100, Frediano Ziglio wrote:
> RedClient was an opaque structure for RedCharDevice.
> It started to be used when RedsState started to contain all
> the global state.
> Make it opaque again using a new RedCharDeviceClientOpaque.
> The RedCharDeviceClientOpaque define in the header allows users
> of the class to override the type to get a more safe type
> than RedClient.
> The define at the beginning of C file is to make sure we don't
> use the opaque type as a specific one.
> 
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>

Acked-by: Victor Toso <victortoso at redhat.com>

> ---
>  server/char-device.c | 52 ++++++++++++++++++++++++--------------------
>  server/char-device.h | 22 +++++++++++--------
>  2 files changed, 42 insertions(+), 32 deletions(-)
> 
> This patch was named "char-device: Make RedClient an opaque structure again"
> 
> diff --git a/server/char-device.c b/server/char-device.c
> index 9ee255664..82255e811 100644
> --- a/server/char-device.c
> +++ b/server/char-device.c
> @@ -22,8 +22,12 @@
>  
>  #include <config.h>
>  #include <inttypes.h>
> +
> +
> +typedef struct RedCharDeviceClientOpaque RedCharDeviceClientOpaque;
> +#define RedCharDeviceClientOpaque RedCharDeviceClientOpaque
> +
>  #include "char-device.h"
> -#include "red-client.h"
>  #include "reds.h"
>  #include "glib-compat.h"
>  
> @@ -39,7 +43,7 @@ typedef enum {
>  } WriteBufferOrigin;
>  
>  struct RedCharDeviceWriteBufferPrivate {
> -    RedClient *client; /* The client that sent the message to the device.
> +    RedCharDeviceClientOpaque *client; /* The client that sent the message to the device.
>                            NULL if the server created the message */
>      WriteBufferOrigin origin;
>      uint32_t token_price;
> @@ -49,7 +53,7 @@ struct RedCharDeviceWriteBufferPrivate {
>  typedef struct RedCharDeviceClient RedCharDeviceClient;
>  struct RedCharDeviceClient {
>      RedCharDevice *dev;
> -    RedClient *client;
> +    RedCharDeviceClientOpaque *client;
>      int do_flow_control;
>      uint64_t num_client_tokens;
>      uint64_t num_client_tokens_free; /* client messages that were consumed by the device */
> @@ -108,7 +112,7 @@ red_char_device_read_one_msg_from_device(RedCharDevice *dev)
>  static void
>  red_char_device_send_msg_to_client(RedCharDevice *dev,
>                                     RedPipeItem *msg,
> -                                   RedClient *client)
> +                                   RedCharDeviceClientOpaque *client)
>  {
>     RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
>  
> @@ -117,7 +121,7 @@ red_char_device_send_msg_to_client(RedCharDevice *dev,
>  
>  static void
>  red_char_device_send_tokens_to_client(RedCharDevice *dev,
> -                                      RedClient *client,
> +                                      RedCharDeviceClientOpaque *client,
>                                        uint32_t tokens)
>  {
>     RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
> @@ -140,7 +144,7 @@ red_char_device_on_free_self_token(RedCharDevice *dev)
>  }
>  
>  static void
> -red_char_device_remove_client(RedCharDevice *dev, RedClient *client)
> +red_char_device_remove_client(RedCharDevice *dev, RedCharDeviceClientOpaque *client)
>  {
>     RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
>  
> @@ -222,7 +226,7 @@ static void red_char_device_handle_client_overflow(RedCharDeviceClient *dev_clie
>  }
>  
>  static RedCharDeviceClient *red_char_device_client_find(RedCharDevice *dev,
> -                                                        RedClient *client)
> +                                                        RedCharDeviceClientOpaque *client)
>  {
>      GList *item;
>  
> @@ -400,7 +404,7 @@ static void red_char_device_send_to_client_tokens_absorb(RedCharDeviceClient *de
>  }
>  
>  void red_char_device_send_to_client_tokens_add(RedCharDevice *dev,
> -                                               RedClient *client,
> +                                               RedCharDeviceClientOpaque *client,
>                                                 uint32_t tokens)
>  {
>      RedCharDeviceClient *dev_client;
> @@ -415,7 +419,7 @@ void red_char_device_send_to_client_tokens_add(RedCharDevice *dev,
>  }
>  
>  void red_char_device_send_to_client_tokens_set(RedCharDevice *dev,
> -                                               RedClient *client,
> +                                               RedCharDeviceClientOpaque *client,
>                                                 uint32_t tokens)
>  {
>      RedCharDeviceClient *dev_client;
> @@ -534,7 +538,7 @@ static void red_char_device_write_retry(void *opaque)
>  }
>  
>  static RedCharDeviceWriteBuffer *
> -red_char_device_write_buffer_get(RedCharDevice *dev, RedClient *client, int size,
> +red_char_device_write_buffer_get(RedCharDevice *dev, RedCharDeviceClientOpaque *client, int size,
>                                   WriteBufferOrigin origin, int migrated_data_tokens)
>  {
>      RedCharDeviceWriteBuffer *ret;
> @@ -598,7 +602,7 @@ error:
>  }
>  
>  RedCharDeviceWriteBuffer *red_char_device_write_buffer_get_client(RedCharDevice *dev,
> -                                                                  RedClient *client,
> +                                                                  RedCharDeviceClientOpaque *client,
>                                                                    int size)
>  {
>      spice_assert(client);
> @@ -658,7 +662,7 @@ void red_char_device_write_buffer_release(RedCharDevice *dev,
>  
>      WriteBufferOrigin buf_origin = write_buf->priv->origin;
>      uint32_t buf_token_price = write_buf->priv->token_price;
> -    RedClient *client = write_buf->priv->client;
> +    RedCharDeviceClientOpaque *client = write_buf->priv->client;
>  
>      if (!dev) {
>          g_warning("no device. write buffer is freed");
> @@ -703,11 +707,13 @@ void red_char_device_destroy(RedCharDevice *char_dev)
>      g_object_unref(char_dev);
>  }
>  
> -static RedCharDeviceClient *red_char_device_client_new(RedClient *client,
> -                                                       int do_flow_control,
> -                                                       uint32_t max_send_queue_size,
> -                                                       uint32_t num_client_tokens,
> -                                                       uint32_t num_send_tokens)
> +static RedCharDeviceClient *
> +red_char_device_client_new(RedsState *reds,
> +                           RedCharDeviceClientOpaque *client,
> +                           int do_flow_control,
> +                           uint32_t max_send_queue_size,
> +                           uint32_t num_client_tokens,
> +                           uint32_t num_send_tokens)
>  {
>      RedCharDeviceClient *dev_client;
>  
> @@ -717,8 +723,6 @@ static RedCharDeviceClient *red_char_device_client_new(RedClient *client,
>      dev_client->max_send_queue_size = max_send_queue_size;
>      dev_client->do_flow_control = do_flow_control;
>      if (do_flow_control) {
> -        RedsState *reds = red_client_get_server(client);
> -
>          dev_client->wait_for_tokens_timer =
>              reds_core_timer_add(reds, device_client_wait_for_tokens_timeout,
>                                  dev_client);
> @@ -736,7 +740,7 @@ static RedCharDeviceClient *red_char_device_client_new(RedClient *client,
>  }
>  
>  bool red_char_device_client_add(RedCharDevice *dev,
> -                                RedClient *client,
> +                                RedCharDeviceClientOpaque *client,
>                                  int do_flow_control,
>                                  uint32_t max_send_queue_size,
>                                  uint32_t num_client_tokens,
> @@ -757,7 +761,9 @@ bool red_char_device_client_add(RedCharDevice *dev,
>      dev->priv->wait_for_migrate_data = wait_for_migrate_data;
>  
>      spice_debug("char device %p, client %p", dev, client);
> -    dev_client = red_char_device_client_new(client, do_flow_control,
> +    dev_client = red_char_device_client_new(dev->priv->reds,
> +                                            client,
> +                                            do_flow_control,
>                                              max_send_queue_size,
>                                              num_client_tokens,
>                                              num_send_tokens);
> @@ -769,7 +775,7 @@ bool red_char_device_client_add(RedCharDevice *dev,
>  }
>  
>  void red_char_device_client_remove(RedCharDevice *dev,
> -                                   RedClient *client)
> +                                   RedCharDeviceClientOpaque *client)
>  {
>      RedCharDeviceClient *dev_client;
>  
> @@ -796,7 +802,7 @@ void red_char_device_client_remove(RedCharDevice *dev,
>  }
>  
>  int red_char_device_client_exists(RedCharDevice *dev,
> -                                  RedClient *client)
> +                                  RedCharDeviceClientOpaque *client)
>  {
>      return (red_char_device_client_find(dev, client) != NULL);
>  }
> diff --git a/server/char-device.h b/server/char-device.h
> index 32ee4f78a..9d4cf9d92 100644
> --- a/server/char-device.h
> +++ b/server/char-device.h
> @@ -38,6 +38,10 @@ typedef struct SpiceCharDeviceState RedCharDevice;
>  typedef struct RedCharDeviceClass RedCharDeviceClass;
>  typedef struct RedCharDevicePrivate RedCharDevicePrivate;
>  
> +#ifndef RedCharDeviceClientOpaque
> +#define RedCharDeviceClientOpaque RedClient
> +#endif
> +
>  /* 'SpiceCharDeviceState' name is used for consistency with what spice-char.h exports */
>  struct SpiceCharDeviceState
>  {
> @@ -62,12 +66,12 @@ struct RedCharDeviceClass
>      /* after this call, the message is unreferenced */
>      void (*send_msg_to_client)(RedCharDevice *self,
>                                 RedPipeItem *msg,
> -                               RedClient *client);
> +                               RedCharDeviceClientOpaque *client);
>  
>      /* The cb is called when a predefined number of write buffers were consumed by the
>       * device */
>      void (*send_tokens_to_client)(RedCharDevice *self,
> -                                  RedClient *client,
> +                                  RedCharDeviceClientOpaque *client,
>                                    uint32_t tokens);
>  
>      /* The cb is called when a server (self) message that was addressed to the device,
> @@ -77,7 +81,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)(RedCharDevice *self, RedClient *client);
> +    void (*remove_client)(RedCharDevice *self, RedCharDeviceClientOpaque *client);
>  
>      /* This cb is called when device receives an event */
>      void (*port_event)(RedCharDevice *self, uint8_t event);
> @@ -188,7 +192,7 @@ void red_char_device_reset(RedCharDevice *dev);
>  /* max_send_queue_size = how many messages we can read from the device and enqueue for this client,
>   * when we have tokens for other clients and no tokens for this one */
>  bool red_char_device_client_add(RedCharDevice *dev,
> -                                RedClient *client,
> +                                RedCharDeviceClientOpaque *client,
>                                  int do_flow_control,
>                                  uint32_t max_send_queue_size,
>                                  uint32_t num_client_tokens,
> @@ -196,9 +200,9 @@ bool red_char_device_client_add(RedCharDevice *dev,
>                                  int wait_for_migrate_data);
>  
>  void red_char_device_client_remove(RedCharDevice *dev,
> -                                   RedClient *client);
> +                                   RedCharDeviceClientOpaque *client);
>  int red_char_device_client_exists(RedCharDevice *dev,
> -                                  RedClient *client);
> +                                  RedCharDeviceClientOpaque *client);
>  
>  void red_char_device_start(RedCharDevice *dev);
>  void red_char_device_stop(RedCharDevice *dev);
> @@ -209,17 +213,17 @@ SpiceServer* red_char_device_get_server(RedCharDevice *dev);
>  void red_char_device_wakeup(RedCharDevice *dev);
>  
>  void red_char_device_send_to_client_tokens_add(RedCharDevice *dev,
> -                                               RedClient *client,
> +                                               RedCharDeviceClientOpaque *client,
>                                                 uint32_t tokens);
>  
>  
>  void red_char_device_send_to_client_tokens_set(RedCharDevice *dev,
> -                                               RedClient *client,
> +                                               RedCharDeviceClientOpaque *client,
>                                                 uint32_t tokens);
>  /** Write to device **/
>  
>  RedCharDeviceWriteBuffer *red_char_device_write_buffer_get_client(RedCharDevice *dev,
> -                                                                  RedClient *client,
> +                                                                  RedCharDeviceClientOpaque *client,
>                                                                    int size);
>  
>  /* Returns NULL if use_token == true and no tokens are available */
> -- 
> 2.20.1
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20190917/78922866/attachment-0001.sig>


More information about the Spice-devel mailing list