Re: [PATCH spice-server v2 2/2] red-channel: Use a single structure to hold capabilities

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 2017-03-01 at 13:21 +0000, Frediano Ziglio wrote:
> Common and channel capabilities are received together
> and copied together.
> As there is already a structure that holds both these capabilities
> use it even for GObject property.
> 
> Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> ---
>  server/cursor-channel-client.c    |  3 +-
>  server/cursor-channel-client.h    |  2 +-
>  server/cursor-channel.c           |  4 +--
>  server/cursor-channel.h           |  2 +-
>  server/dcc.c                      |  3 +-
>  server/dcc.h                      |  2 +-
>  server/inputs-channel-client.c    |  3 +-
>  server/inputs-channel-client.h    |  2 +-
>  server/inputs-channel.c           |  5 ++--
>  server/main-channel-client.c      |  3 +-
>  server/main-channel-client.h      |  2 +-
>  server/main-channel.c             |  5 ++--
>  server/main-channel.h             |  2 +-
>  server/red-channel-client.c       | 51 ++++-------------------------
> -----
>  server/red-channel-client.h       |  2 +-
>  server/red-channel.c              | 58
> ++++++++++++++++++++++++++++++++++++---
>  server/red-channel.h              | 14 +++++++---
>  server/red-qxl.c                  | 10 +++----
>  server/red-qxl.h                  |  6 ++--
>  server/red-worker.c               | 10 +++----
>  server/reds.c                     | 49 ++++++++++++++---------------
> ----
>  server/smartcard-channel-client.c |  4 +--
>  server/smartcard-channel-client.h |  3 +-
>  server/smartcard.c                |  3 +-
>  server/sound.c                    | 13 ++++-----
>  server/spicevmc.c                 |  7 ++---
>  26 files changed, 129 insertions(+), 139 deletions(-)
> 
> diff --git a/server/cursor-channel-client.c b/server/cursor-channel-
> client.c
> index 1344719..1a05f73 100644
> --- a/server/cursor-channel-client.c
> +++ b/server/cursor-channel-client.c
> @@ -93,7 +93,7 @@ void cursor_channel_client_migrate(RedChannelClient
> *rcc)
>  
>  CursorChannelClient* cursor_channel_client_new(CursorChannel
> *cursor, RedClient *client, RedsStream *stream,
>                                                 int mig_target,
> -                                               GArray *common_caps,
> GArray *caps)
> +                                               RedChannelCapabilitie
> s *caps)
>  {
>      CursorChannelClient *rcc;
>  
> @@ -103,7 +103,6 @@ CursorChannelClient*
> cursor_channel_client_new(CursorChannel *cursor, RedClient
>                           "client", client,
>                           "stream", stream,
>                           "monitor-latency", FALSE,
> -                         "common-caps", common_caps,
>                           "caps", caps,
>                           NULL);
>      common_graphics_channel_set_during_target_migrate(COMMON_GRAPHIC
> S_CHANNEL(cursor), mig_target);
> diff --git a/server/cursor-channel-client.h b/server/cursor-channel-
> client.h
> index 211ea94..e2aa3a8 100644
> --- a/server/cursor-channel-client.h
> +++ b/server/cursor-channel-client.h
> @@ -63,7 +63,7 @@ CursorChannelClient*
> cursor_channel_client_new(CursorChannel *cursor,
>                                                 RedClient *client,
>                                                 RedsStream *stream,
>                                                 int mig_target,
> -                                               GArray *common_caps,
> GArray *caps);
> +                                               RedChannelCapabilitie
> s *caps);
>  
>  void cursor_channel_client_reset_cursor_cache(RedChannelClient
> *rcc);
>  void cursor_channel_client_on_disconnect(RedChannelClient *rcc);
> diff --git a/server/cursor-channel.c b/server/cursor-channel.c
> index 73d7126..5b23a16 100644
> --- a/server/cursor-channel.c
> +++ b/server/cursor-channel.c
> @@ -406,7 +406,7 @@ void cursor_channel_set_mouse_mode(CursorChannel
> *cursor, uint32_t mode)
>  
>  void cursor_channel_connect(CursorChannel *cursor, RedClient
> *client, RedsStream *stream,
>                              int migrate,
> -                            GArray *common_caps, GArray *caps)
> +                            RedChannelCapabilities *caps)
>  {
>      CursorChannelClient *ccc;
>  
> @@ -415,7 +415,7 @@ void cursor_channel_connect(CursorChannel
> *cursor, RedClient *client, RedsStream
>      spice_info("add cursor channel client");
>      ccc = cursor_channel_client_new(cursor, client, stream,
>                                      migrate,
> -                                    common_caps, caps);
> +                                    caps);
>      spice_return_if_fail(ccc != NULL);
>  
>      RedChannelClient *rcc = RED_CHANNEL_CLIENT(ccc);
> diff --git a/server/cursor-channel.h b/server/cursor-channel.h
> index 2c3948d..abf7b3d 100644
> --- a/server/cursor-channel.h
> +++ b/server/cursor-channel.h
> @@ -75,7 +75,7 @@
> void                 cursor_channel_set_mouse_mode(CursorChannel
> *cursor, uint32
>  void cursor_channel_connect(CursorChannel *cursor, RedClient
> *client,
>                              RedsStream *stream,
>                              int migrate,
> -                            GArray *common_caps, GArray *caps);
> +                            RedChannelCapabilities *caps);
>  
>  /**
>   * Migrate a client channel from a CursorChannel.
> diff --git a/server/dcc.c b/server/dcc.c
> index 3cc9573..6c1c89e 100644
> --- a/server/dcc.c
> +++ b/server/dcc.c
> @@ -482,7 +482,7 @@ static void
> dcc_init_stream_agents(DisplayChannelClient *dcc)
>  DisplayChannelClient *dcc_new(DisplayChannel *display,
>                                RedClient *client, RedsStream *stream,
>                                int mig_target,
> -                              GArray *common_caps, GArray *caps,
> +                              RedChannelCapabilities *caps,
>                                SpiceImageCompression
> image_compression,
>                                spice_wan_compression_t jpeg_state,
>                                spice_wan_compression_t
> zlib_glz_state)
> @@ -496,7 +496,6 @@ DisplayChannelClient *dcc_new(DisplayChannel
> *display,
>                           "client", client,
>                           "stream", stream,
>                           "monitor-latency", TRUE,
> -                         "common-caps", common_caps,
>                           "caps", caps,
>                           "image-compression", image_compression,
>                           "jpeg-state", jpeg_state,
> diff --git a/server/dcc.h b/server/dcc.h
> index 19416e3..9a82a5d 100644
> --- a/server/dcc.h
> +++ b/server/dcc.h
> @@ -131,7 +131,7 @@ typedef struct RedDrawablePipeItem {
>  } RedDrawablePipeItem;
>  
>  DisplayChannelClient* dcc_new(DisplayChannel *display, RedClient
> *client, RedsStream *stream,
> -                              int mig_target, GArray *common_caps,
> GArray *caps,
> +                              int mig_target, RedChannelCapabilities
> *caps,
>                                SpiceImageCompression
> image_compression,
>                                spice_wan_compression_t jpeg_state,
>                                spice_wan_compression_t
> zlib_glz_state);
> diff --git a/server/inputs-channel-client.c b/server/inputs-channel-
> client.c
> index c8d33ba..72b5c39 100644
> --- a/server/inputs-channel-client.c
> +++ b/server/inputs-channel-client.c
> @@ -48,7 +48,7 @@ RedChannelClient*
> inputs_channel_client_create(RedChannel *channel,
>                                                 RedClient *client,
>                                                 RedsStream *stream,
>                                                 int monitor_latency,
> -                                               GArray *common_caps,
> GArray *caps)
> +                                               RedChannelCapabilitie
> s *caps)
>  {
>      RedChannelClient *rcc;
>  
> @@ -59,7 +59,6 @@ RedChannelClient*
> inputs_channel_client_create(RedChannel *channel,
>                           "stream", stream,
>                           "monitor-latency", monitor_latency,
>                           "caps", caps,
> -                         "common-caps", common_caps,
>                           NULL);
>  
>      return rcc;
> diff --git a/server/inputs-channel-client.h b/server/inputs-channel-
> client.h
> index a856a58..ba08dbf 100644
> --- a/server/inputs-channel-client.h
> +++ b/server/inputs-channel-client.h
> @@ -60,7 +60,7 @@ RedChannelClient*
> inputs_channel_client_create(RedChannel *channel,
>                                                 RedClient *client,
>                                                 RedsStream *stream,
>                                                 int monitor_latency,
> -                                               GArray *common_caps,
> GArray *caps);
> +                                               RedChannelCapabilitie
> s *caps);
>  
>  uint16_t inputs_channel_client_get_motion_count(InputsChannelClient*
> self);
>  /* only for migration */
> diff --git a/server/inputs-channel.c b/server/inputs-channel.c
> index cd92e1a..ec297a2 100644
> --- a/server/inputs-channel.c
> +++ b/server/inputs-channel.c
> @@ -486,7 +486,7 @@ static void inputs_pipe_add_init(RedChannelClient
> *rcc)
>  
>  static void inputs_connect(RedChannel *channel, RedClient *client,
>                             RedsStream *stream, int migration,
> -                           GArray *common_caps, GArray *caps)
> +                           RedChannelCapabilities *caps)
>  {
>      RedChannelClient *rcc;
>  
> @@ -496,8 +496,7 @@ static void inputs_connect(RedChannel *channel,
> RedClient *client,
>      }
>  
>      spice_printerr("inputs channel client create");
> -    rcc = inputs_channel_client_create(channel, client, stream,
> FALSE,
> -                                       common_caps, caps);
> +    rcc = inputs_channel_client_create(channel, client, stream,
> FALSE, caps);
>      if (!rcc) {
>          return;
>      }
> diff --git a/server/main-channel-client.c b/server/main-channel-
> client.c
> index c8e01bc..2b68407 100644
> --- a/server/main-channel-client.c
> +++ b/server/main-channel-client.c
> @@ -651,7 +651,7 @@ static void ping_timer_cb(void *opaque)
>  
>  MainChannelClient *main_channel_client_create(MainChannel
> *main_chan, RedClient *client,
>                                                RedsStream *stream,
> uint32_t connection_id,
> -                                              GArray *common_caps,
> GArray *caps)
> +                                              RedChannelCapabilities
> *caps)
>  {
>      MainChannelClient *mcc;
>  
> @@ -662,7 +662,6 @@ MainChannelClient
> *main_channel_client_create(MainChannel *main_chan, RedClient
>                           "stream", stream,
>                           "monitor-latency", FALSE,
>                           "caps", caps,
> -                         "common-caps", common_caps,
>                           "connection-id", connection_id,
>                           NULL);
>  
> diff --git a/server/main-channel-client.h b/server/main-channel-
> client.h
> index 5cb2aad..9c7009d 100644
> --- a/server/main-channel-client.h
> +++ b/server/main-channel-client.h
> @@ -58,7 +58,7 @@ GType main_channel_client_get_type(void)
> G_GNUC_CONST;
>  
>  MainChannelClient *main_channel_client_create(MainChannel
> *main_chan, RedClient *client,
>                                                RedsStream *stream,
> uint32_t connection_id,
> -                                              GArray *common_caps,
> GArray *caps);
> +                                              RedChannelCapabilities
> *caps);
>  
>  void main_channel_client_push_agent_tokens(MainChannelClient *mcc,
> uint32_t num_tokens);
>  void main_channel_client_push_agent_data(MainChannelClient *mcc,
> uint8_t* data, size_t len,
> diff --git a/server/main-channel.c b/server/main-channel.c
> index 7912ced..307c80f 100644
> --- a/server/main-channel.c
> +++ b/server/main-channel.c
> @@ -287,7 +287,7 @@ static int
> main_channel_handle_migrate_flush_mark(RedChannelClient *rcc)
>  
>  MainChannelClient *main_channel_link(MainChannel *channel, RedClient
> *client,
>                                       RedsStream *stream, uint32_t
> connection_id, int migration,
> -                                     GArray *common_caps, GArray
> *caps)
> +                                     RedChannelCapabilities *caps)
>  {
>      MainChannelClient *mcc;
>  
> @@ -297,8 +297,7 @@ MainChannelClient *main_channel_link(MainChannel
> *channel, RedClient *client,
>      // into usage somewhere (not an issue until we return migration
> to it's
>      // former glory)
>      spice_printerr("add main channel client");
> -    mcc = main_channel_client_create(channel, client, stream,
> connection_id,
> -                                     common_caps, caps);
> +    mcc = main_channel_client_create(channel, client, stream,
> connection_id, caps);
>      return mcc;
>  }
>  
> diff --git a/server/main-channel.h b/server/main-channel.h
> index 5992f1c..0209024 100644
> --- a/server/main-channel.h
> +++ b/server/main-channel.h
> @@ -60,7 +60,7 @@ RedClient
> *main_channel_get_client_by_link_id(MainChannel *main_chan, uint32_t
> l
>  /* This is a 'clone' from the reds.h Channel.link callback to allow
> passing link_id */
>  MainChannelClient *main_channel_link(MainChannel *, RedClient
> *client,
>       RedsStream *stream, uint32_t link_id, int migration,
> -     GArray *common_caps, GArray *caps);
> +     RedChannelCapabilities *caps);
>  void main_channel_push_mouse_mode(MainChannel *main_chan, int
> current_mode, int is_client_mouse_allowed);
>  void main_channel_push_agent_connected(MainChannel *main_chan);
>  void main_channel_push_agent_disconnected(MainChannel *main_chan);
> diff --git a/server/red-channel-client.c b/server/red-channel-
> client.c
> index 0100af7..a9917db 100644
> --- a/server/red-channel-client.c
> +++ b/server/red-channel-client.c
> @@ -188,7 +188,6 @@ enum {
>      PROP_CHANNEL,
>      PROP_CLIENT,
>      PROP_MONITOR_LATENCY,
> -    PROP_COMMON_CAPS,
>      PROP_CAPS
>  };
>  
> @@ -286,22 +285,6 @@ red_channel_client_get_property(GObject *object,
>          case PROP_MONITOR_LATENCY:
>              g_value_set_boolean(value, self->priv->monitor_latency);
>              break;
> -        case PROP_COMMON_CAPS:
> -            {
> -                GArray *arr = g_array_sized_new(FALSE, FALSE,
> -                                                sizeof(*self->priv-
> >remote_caps.common_caps),
> -                                                self->priv-
> >remote_caps.num_common_caps);
> -                g_value_take_boxed(value, arr);
> -            }
> -            break;
> -        case PROP_CAPS:
> -            {
> -                GArray *arr = g_array_sized_new(FALSE, FALSE,
> -                                                sizeof(*self->priv-
> >remote_caps.caps),
> -                                                self->priv-
> >remote_caps.num_caps);
> -                g_value_take_boxed(value, arr);
> -            }
> -            break;
>          default:
>              G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id,
> pspec);
>      }
> @@ -331,25 +314,12 @@ red_channel_client_set_property(GObject
> *object,
>          case PROP_MONITOR_LATENCY:
>              self->priv->monitor_latency =
> g_value_get_boolean(value);
>              break;
> -        case PROP_COMMON_CAPS:
> -            {
> -                GArray *caps = g_value_get_boxed(value);
> -                if (caps) {
> -                    self->priv->remote_caps.num_common_caps = caps-
> >len;
> -                    free(self->priv->remote_caps.common_caps);
> -                    self->priv->remote_caps.common_caps =
> -                        spice_memdup(caps->data, caps->len *
> sizeof(uint32_t));
> -                }
> -            }
> -            break;
>          case PROP_CAPS:
>              {
> -                GArray *caps = g_value_get_boxed(value);
> +                RedChannelCapabilities *caps =
> g_value_get_boxed(value);
>                  if (caps) {
> -                    self->priv->remote_caps.num_caps = caps->len;
> -                    free(self->priv->remote_caps.caps);
> -                    self->priv->remote_caps.caps =
> -                        spice_memdup(caps->data, caps->len *
> sizeof(uint32_t));
> +                    red_channel_capabilities_deinit(&self->priv-
> >remote_caps);
> +                    red_channel_capabilities_init(&self->priv-
> >remote_caps, caps);
>                  }
>              }
>              break;
> @@ -450,19 +420,11 @@ static void
> red_channel_client_class_init(RedChannelClientClass *klass)
>                                  | G_PARAM_CONSTRUCT_ONLY);
>      g_object_class_install_property(object_class,
> PROP_MONITOR_LATENCY, spec);
>  
> -    spec = g_param_spec_boxed("common-caps", "common-caps",
> -                              "Common Capabilities",
> -                              G_TYPE_ARRAY,
> -                              G_PARAM_STATIC_STRINGS
> -                              | G_PARAM_READWRITE
> -                              | G_PARAM_CONSTRUCT_ONLY);
> -    g_object_class_install_property(object_class, PROP_COMMON_CAPS,
> spec);
> -
>      spec = g_param_spec_boxed("caps", "caps",
>                                "Capabilities",
> -                              G_TYPE_ARRAY,
> +                              red_channel_capabilities_type,
>                                G_PARAM_STATIC_STRINGS
> -                              | G_PARAM_READWRITE
> +                              | G_PARAM_WRITABLE


I think it would be worth mentioning this read/write access change in
the commit log. Or maybe even separating it out into a separate commit
before this one?

>                                | G_PARAM_CONSTRUCT_ONLY);
>      g_object_class_install_property(object_class, PROP_CAPS, spec);
>  }
> @@ -1013,7 +975,7 @@ cleanup:
>  RedChannelClient *red_channel_client_create(RedChannel *channel,
> RedClient *client,
>                                              RedsStream *stream,
>                                              int monitor_latency,
> -                                            GArray *common_caps,
> GArray *caps)
> +                                            RedChannelCapabilities
> *caps)
>  {
>      RedChannelClient *rcc;
>  
> @@ -1024,7 +986,6 @@ RedChannelClient
> *red_channel_client_create(RedChannel *channel, RedClient *clie
>                           "stream", stream,
>                           "monitor-latency", monitor_latency,
>                           "caps", caps,
> -                         "common-caps", common_caps,
>                           NULL);
>  
>      return rcc;
> diff --git a/server/red-channel-client.h b/server/red-channel-
> client.h
> index f5be4a1..1450089 100644
> --- a/server/red-channel-client.h
> +++ b/server/red-channel-client.h
> @@ -50,7 +50,7 @@ GType red_channel_client_get_type(void)
> G_GNUC_CONST;
>  RedChannelClient *red_channel_client_create(RedChannel *channel,
>                                              RedClient *client,
> RedsStream *stream,
>                                              int monitor_latency,
> -                                            GArray *common_caps,
> GArray *caps);
> +                                            RedChannelCapabilities
> *caps);
>  
>  gboolean red_channel_client_is_connected(RedChannelClient *rcc);
>  void red_channel_client_default_migrate(RedChannelClient *rcc);
> diff --git a/server/red-channel.c b/server/red-channel.c
> index 56fa4fc..0ebb8e2 100644
> --- a/server/red-channel.c
> +++ b/server/red-channel.c
> @@ -219,7 +219,7 @@ red_channel_constructed(GObject *object)
>  static void red_channel_client_default_connect(RedChannel *channel,
> RedClient *client,
>                                                 RedsStream *stream,
>                                                 int migration,
> -                                               GArray *common_caps,
> GArray *caps)
> +                                               RedChannelCapabilitie
> s *caps)
>  {
>      spice_error("not implemented");
>  }
> @@ -401,6 +401,57 @@ void red_channel_register_client_cbs(RedChannel
> *channel, const ClientCbs *clien
>      channel->priv->data = cbs_data;
>  }
>  
> +void red_channel_capabilities_init(RedChannelCapabilities *dest,
> +                                   const RedChannelCapabilities
> *caps)
> +{
> +    *dest = *caps;
> +    if (caps->common_caps) {
> +        dest->common_caps = spice_memdup(caps->common_caps,
> +                                         caps->num_common_caps *
> sizeof(uint32_t));
> +    }
> +    if (caps->num_caps) {
> +        dest->caps = spice_memdup(caps->caps, caps->num_caps *
> sizeof(uint32_t));
> +    }
> +}
> +
> +void red_channel_capabilities_deinit(RedChannelCapabilities *caps)

Personally, I would prefer the name _reset() to _init()

> +{
> +    if (caps->num_common_caps) {
> +        free(caps->common_caps);
> +    }
> +
> +    if (caps->num_caps) {
> +        free(caps->caps);
> +    }
> +}

I would also prefer to reset all members to 0/NULL in this function so
that we don't leave it in a inconsistent state. It makes it easier to
double-free in the future.

In addition, now that we have this function (especially if we make the
changes I suggested above), we no longer need the function
red_channel_client_destroy_remote_caps(). We can also use this function
within red_channel_finalize() to free the local_caps.

> +
> +static RedChannelCapabilities *red_channel_capabilities_dup(const
> RedChannelCapabilities *caps)
> +{
> +    if (!caps) {
> +        return NULL;
> +    }
> +
> +    RedChannelCapabilities *res = spice_new(RedChannelCapabilities,
> 1);
> +    red_channel_capabilities_init(res, caps);
> +    return res;
> +}
> +
> +static void red_channel_capabilities_destroy(RedChannelCapabilities
> *caps)
> +{
> +    red_channel_capabilities_deinit(caps);
> +    free(caps);
> +}
> +
> +GType red_channel_capabilities_type;
> +
> +SPICE_CONSTRUCTOR_FUNC(red_channel_capabilities_construct)
> +{
> +    red_channel_capabilities_type =
> +        g_boxed_type_register_static("red_channel_capabilities_type"
> ,
> +                                     (GBoxedCopyFunc)
> red_channel_capabilities_dup,
> +                                     (GBoxedFreeFunc)
> red_channel_capabilities_destroy);
> +}
> +
>  static void add_capability(uint32_t **caps, int *num_caps, uint32_t
> cap)
>  {
>      int nbefore, n;
> @@ -513,10 +564,9 @@ void red_channel_disconnect(RedChannel *channel)
>  
>  void red_channel_connect(RedChannel *channel, RedClient *client,
>                           RedsStream *stream, int migration,
> -                         GArray *common_caps, GArray *caps)
> +                         RedChannelCapabilities *caps)
>  {
> -    channel->priv->client_cbs.connect(channel, client, stream,
> migration,
> -                                      common_caps, caps);
> +    channel->priv->client_cbs.connect(channel, client, stream,
> migration, caps);
>  }
>  
>  void red_channel_apply_clients(RedChannel *channel,
> channel_client_callback cb)
> diff --git a/server/red-channel.h b/server/red-channel.h
> index b20e254..251efc7 100644
> --- a/server/red-channel.h
> +++ b/server/red-channel.h
> @@ -41,6 +41,7 @@ typedef struct RedChannel RedChannel;
>  typedef struct RedChannelClient RedChannelClient;
>  typedef struct RedClient RedClient;
>  typedef struct MainChannelClient MainChannelClient;
> +typedef struct RedChannelCapabilities RedChannelCapabilities;
>  
>  typedef uint8_t *(*channel_alloc_msg_recv_buf_proc)(RedChannelClient
> *channel,
>                                                      uint16_t type,
> uint32_t size);
> @@ -60,7 +61,7 @@ typedef uint64_t
> (*channel_handle_migrate_data_get_serial_proc)(RedChannelClient
>  
>  
>  typedef void (*channel_client_connect_proc)(RedChannel *channel,
> RedClient *client, RedsStream *stream,
> -                                            int migration, GArray
> *common_caps, GArray *caps);
> +                                            int migration,
> RedChannelCapabilities *caps);
>  typedef void (*channel_client_disconnect_proc)(RedChannelClient
> *base);
>  typedef void (*channel_client_migrate_proc)(RedChannelClient *base);
>  
> @@ -134,12 +135,17 @@ struct RedChannelClass
>  
>  /* Red Channel interface */
>  
> -typedef struct RedChannelCapabilities {
> +struct RedChannelCapabilities {
>      int num_common_caps;
>      uint32_t *common_caps;
>      int num_caps;
>      uint32_t *caps;
> -} RedChannelCapabilities;
> +};
> +
> +void red_channel_capabilities_init(RedChannelCapabilities *dest,
> +                                   const RedChannelCapabilities
> *caps);
> +void red_channel_capabilities_deinit(RedChannelCapabilities *caps);
> +extern GType red_channel_capabilities_type;
>  
>  GType red_channel_get_type(void) G_GNUC_CONST;
>  
> @@ -215,7 +221,7 @@ void red_channel_send(RedChannel *channel);
>  void red_channel_disconnect(RedChannel *channel);
>  void red_channel_connect(RedChannel *channel, RedClient *client,
>                           RedsStream *stream, int migration,
> -                         GArray *common_caps, GArray *caps);
> +                         RedChannelCapabilities *caps);
>  
>  /* return the sum of all the rcc pipe size */
>  uint32_t red_channel_max_pipe_size(RedChannel *channel);
> diff --git a/server/red-qxl.c b/server/red-qxl.c
> index 12ae87f..53f3338 100644
> --- a/server/red-qxl.c
> +++ b/server/red-qxl.c
> @@ -76,7 +76,7 @@ int red_qxl_check_qxl_version(QXLInstance *qxl, int
> major, int minor)
>  
>  static void red_qxl_set_display_peer(RedChannel *channel, RedClient
> *client,
>                                       RedsStream *stream, int
> migration,
> -                                     GArray *common_caps, GArray
> *caps)
> +                                     RedChannelCapabilities *caps)
>  {
>      RedWorkerMessageDisplayConnect payload = {0,};
>      Dispatcher *dispatcher;
> @@ -86,8 +86,7 @@ static void red_qxl_set_display_peer(RedChannel
> *channel, RedClient *client,
>      payload.client = client;
>      payload.stream = stream;
>      payload.migration = migration;
> -    payload.common_caps = g_array_ref(common_caps);
> -    payload.caps = g_array_ref(caps);
> +    red_channel_capabilities_init(&payload.caps, caps);
>  
>      dispatcher_send_message(dispatcher,
>                              RED_WORKER_MESSAGE_DISPLAY_CONNECT,
> @@ -137,7 +136,7 @@ static void
> red_qxl_display_migrate(RedChannelClient *rcc)
>  
>  static void red_qxl_set_cursor_peer(RedChannel *channel, RedClient
> *client, RedsStream *stream,
>                                      int migration,
> -                                    GArray *common_caps, GArray
> *caps)
> +                                    RedChannelCapabilities *caps)
>  {
>      RedWorkerMessageCursorConnect payload = {0,};
>      Dispatcher *dispatcher = (Dispatcher
> *)g_object_get_data(G_OBJECT(channel), "dispatcher");
> @@ -145,8 +144,7 @@ static void red_qxl_set_cursor_peer(RedChannel
> *channel, RedClient *client, Reds
>      payload.client = client;
>      payload.stream = stream;
>      payload.migration = migration;
> -    payload.common_caps = g_array_ref(common_caps);
> -    payload.caps = g_array_ref(caps);
> +    red_channel_capabilities_init(&payload.caps, caps);
>  
>      dispatcher_send_message(dispatcher,
>                              RED_WORKER_MESSAGE_CURSOR_CONNECT,
> diff --git a/server/red-qxl.h b/server/red-qxl.h
> index 4d3c3de..80a2de3 100644
> --- a/server/red-qxl.h
> +++ b/server/red-qxl.h
> @@ -126,8 +126,7 @@ enum {
>  typedef struct RedWorkerMessageDisplayConnect {
>      RedClient * client;
>      RedsStream * stream;
> -    GArray *common_caps; // red_worker should release
> -    GArray *caps;        // red_worker should release
> +    RedChannelCapabilities caps;        // red_worker should deinit
>      int migration;
>  } RedWorkerMessageDisplayConnect;
>  
> @@ -143,8 +142,7 @@ typedef struct RedWorkerMessageCursorConnect {
>      RedClient *client;
>      RedsStream *stream;
>      int migration;
> -    GArray *common_caps; // red_worker should release
> -    GArray *caps;        // red_worker should release
> +    RedChannelCapabilities caps;        // red_worker should deinit
>  } RedWorkerMessageCursorConnect;
>  
>  typedef struct RedWorkerMessageCursorDisconnect {
> diff --git a/server/red-worker.c b/server/red-worker.c
> index 779a6c1..30bf19a 100644
> --- a/server/red-worker.c
> +++ b/server/red-worker.c
> @@ -736,7 +736,7 @@ static void handle_dev_display_connect(void
> *opaque, void *payload)
>      spice_return_if_fail(display);
>  
>      dcc = dcc_new(display, msg->client, msg->stream, msg->migration,
> -                  msg->common_caps, msg->caps,
> +                  &msg->caps,
>                    worker->image_compression, worker->jpeg_state,
> worker->zlib_glz_state);
>      if (!dcc) {
>          return;
> @@ -745,8 +745,7 @@ static void handle_dev_display_connect(void
> *opaque, void *payload)
>      guest_set_client_capabilities(worker);
>      dcc_start(dcc);
>  
> -    g_array_unref(msg->caps);
> -    g_array_unref(msg->common_caps);
> +    red_channel_capabilities_deinit(&msg->caps);
>  }
>  
>  static void handle_dev_display_disconnect(void *opaque, void
> *payload)
> @@ -832,9 +831,8 @@ static void handle_dev_cursor_connect(void
> *opaque, void *payload)
>      spice_info("cursor connect");
>      cursor_channel_connect(worker->cursor_channel,
>                             msg->client, msg->stream, msg->migration,
> -                           msg->common_caps, msg->caps);
> -    g_array_unref(msg->caps);
> -    g_array_unref(msg->common_caps);
> +                           &msg->caps);
> +    red_channel_capabilities_deinit(&msg->caps);
>  }
>  
>  static void handle_dev_cursor_disconnect(void *opaque, void
> *payload)
> diff --git a/server/reds.c b/server/reds.c
> index ff98a48..a114a24 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -1675,22 +1675,20 @@ static RedClient *reds_get_client(RedsState
> *reds)
>      return reds->clients->data;
>  }
>  
> -static GArray *reds_caps_new(const uint32_t *caps, size_t num_caps)
> +static void
> +red_channel_capabilities_from_link(RedChannelCapabilities *caps,
> const SpiceLinkMess *link_mess)
>  {
> -    if (caps == NULL || num_caps == 0) {
> -        return NULL;
> -    }
> +    uint32_t *raw_caps = (uint32_t *)((uint8_t *)link_mess +
> link_mess->caps_offset);
>  
> -    GArray *array = g_array_sized_new(FALSE, FALSE, sizeof (*caps),
> num_caps);
> -    g_array_append_vals(array, caps, num_caps);
> -    return array;
> -}
> -
> -static inline void reds_caps_unref(GArray *caps)
> -{
> -    if (caps) {
> -        g_array_unref(caps);
> -    }
> +    caps->num_common_caps = link_mess->num_common_caps;
> +    caps->common_caps =
> +        link_mess->num_common_caps ?
> +        spice_memdup(raw_caps, link_mess->num_common_caps *
> sizeof(uint32_t)) : NULL;
> +    caps->num_caps = link_mess->num_channel_caps;
> +    caps->caps =
> +        link_mess->num_channel_caps ?
> +        spice_memdup(raw_caps + link_mess->num_common_caps,
> +                     link_mess->num_channel_caps * sizeof(uint32_t))
> : NULL;
>  }
>  
>  // TODO: now that main is a separate channel this should
> @@ -1700,10 +1698,10 @@ static void reds_handle_main_link(RedsState
> *reds, RedLinkInfo *link)
>      RedClient *client;
>      RedsStream *stream;
>      SpiceLinkMess *link_mess;
> -    uint32_t *caps;
>      uint32_t connection_id;
>      MainChannelClient *mcc;
>      int mig_target = FALSE;
> +    RedChannelCapabilities caps;
>  
>      spice_info(NULL);
>      spice_assert(reds->main_channel);
> @@ -1735,16 +1733,14 @@ static void reds_handle_main_link(RedsState
> *reds, RedLinkInfo *link)
>      link->stream = NULL;
>      link->link_mess = NULL;
>      reds_link_free(link);
> -    caps = (uint32_t *)((uint8_t *)link_mess + link_mess-
> >caps_offset);
>      client = red_client_new(reds, mig_target);
>      reds->clients = g_list_prepend(reds->clients, client);
> -    GArray *common_caps = reds_caps_new(caps, link_mess-
> >num_common_caps);
> -    GArray *channel_caps = reds_caps_new(caps + link_mess-
> >num_common_caps, link_mess->num_channel_caps);
> +
> +    red_channel_capabilities_from_link(&caps, link_mess);
>      mcc = main_channel_link(reds->main_channel, client,
>                              stream, connection_id, mig_target,
> -                            common_caps, channel_caps);
> -    reds_caps_unref(common_caps);
> -    reds_caps_unref(channel_caps);
> +                            &caps);
> +    red_channel_capabilities_deinit(&caps);
>      spice_info("NEW Client %p mcc %p connect-id %d", client, mcc,
> connection_id);
>      free(link_mess);
>      red_client_set_main(client, mcc);
> @@ -1804,20 +1800,17 @@ static void reds_channel_do_link(RedChannel
> *channel, RedClient *client,
>                                   SpiceLinkMess *link_msg,
>                                   RedsStream *stream)
>  {
> -    uint32_t *caps;
> +    RedChannelCapabilities caps;
>  
>      spice_assert(channel);
>      spice_assert(link_msg);
>      spice_assert(stream);
>  
> -    caps = (uint32_t *)((uint8_t *)link_msg + link_msg-
> >caps_offset);
> -    GArray *common_caps = reds_caps_new(caps, link_msg-
> >num_common_caps);
> -    GArray *channel_caps = reds_caps_new(caps + link_msg-
> >num_common_caps, link_msg->num_channel_caps);
> +    red_channel_capabilities_from_link(&caps, link_msg);
>      red_channel_connect(channel, client, stream,
>                          red_client_during_migrate_at_target(client),
> -                        common_caps, channel_caps);
> -    reds_caps_unref(common_caps);
> -    reds_caps_unref(channel_caps);
> +                        &caps);
> +    red_channel_capabilities_deinit(&caps);
>  }
>  
>  /*
> diff --git a/server/smartcard-channel-client.c b/server/smartcard-
> channel-client.c
> index 2c99309..04c5c04 100644
> --- a/server/smartcard-channel-client.c
> +++ b/server/smartcard-channel-client.c
> @@ -103,8 +103,7 @@
> smart_card_channel_client_init(SmartCardChannelClient *self)
>  SmartCardChannelClient* smartcard_channel_client_create(RedChannel
> *channel,
>                                                          RedClient
> *client, RedsStream *stream,
>                                                          int
> monitor_latency,
> -                                                        GArray
> *common_caps,
> -                                                        GArray
> *caps)
> +                                                        RedChannelCa
> pabilities *caps)
>  {
>      SmartCardChannelClient *rcc;
>  
> @@ -115,7 +114,6 @@ SmartCardChannelClient*
> smartcard_channel_client_create(RedChannel *channel,
>                           "stream", stream,
>                           "monitor-latency", monitor_latency,
>                           "caps", caps,
> -                         "common-caps", common_caps,
>                           NULL);
>  
>      return rcc;
> diff --git a/server/smartcard-channel-client.h b/server/smartcard-
> channel-client.h
> index 56e36ec..84181a4 100644
> --- a/server/smartcard-channel-client.h
> +++ b/server/smartcard-channel-client.h
> @@ -58,8 +58,7 @@ GType smart_card_channel_client_get_type(void)
> G_GNUC_CONST;
>  SmartCardChannelClient* smartcard_channel_client_create(RedChannel
> *channel,
>                                                          RedClient
> *client, RedsStream *stream,
>                                                          int
> monitor_latency,
> -                                                        GArray
> *common_caps,
> -                                                        GArray
> *caps);
> +                                                        RedChannelCa
> pabilities *caps);
>  
>  uint8_t* smartcard_channel_client_alloc_msg_rcv_buf(RedChannelClient
> *rcc,
>                                                      uint16_t type,
> diff --git a/server/smartcard.c b/server/smartcard.c
> index b444e4d..7b8ad01 100644
> --- a/server/smartcard.c
> +++ b/server/smartcard.c
> @@ -528,7 +528,7 @@ int
> smartcard_char_device_handle_migrate_data(RedCharDeviceSmartcard
> *smartcard,
>  
>  static void smartcard_connect_client(RedChannel *channel, RedClient
> *client,
>                                       RedsStream *stream, int
> migration,
> -                                     GArray *common_caps, GArray
> *caps)
> +                                     RedChannelCapabilities *caps)
>  {
>      SpiceCharDeviceInstance *char_device =
>              smartcard_readers_get_unattached();
> @@ -539,7 +539,6 @@ static void smartcard_connect_client(RedChannel
> *channel, RedClient *client,
>                                            client,
>                                            stream,
>                                            FALSE,
> -                                          common_caps,
>                                            caps);
>  
>      if (!scc) {
> diff --git a/server/sound.c b/server/sound.c
> index e41ab1e..a227daf 100644
> --- a/server/sound.c
> +++ b/server/sound.c
> @@ -1079,7 +1079,7 @@ playback_channel_client_constructed(GObject
> *object)
>  }
>  
>  static void snd_set_peer(RedChannel *red_channel, RedClient *client,
> RedsStream *stream,
> -                         GArray *common_caps, GArray *caps, GType
> type)
> +                         RedChannelCapabilities *caps, GType type)
>  {
>      SndChannel *channel = SND_CHANNEL(red_channel);
>      SndChannelClient *snd_client;
> @@ -1095,17 +1095,15 @@ static void snd_set_peer(RedChannel
> *red_channel, RedClient *client, RedsStream
>                                  "client", client,
>                                  "stream", stream,
>                                  "caps", caps,
> -                                "common-caps", common_caps,
>                                  NULL);
>      g_warn_if_fail(snd_client != NULL);
>  }
>  
>  static void snd_set_playback_peer(RedChannel *red_channel, RedClient
> *client, RedsStream *stream,
>                                    G_GNUC_UNUSED int migration,
> -                                  GArray *common_caps, GArray *caps)
> +                                  RedChannelCapabilities *caps)
>  {
> -    snd_set_peer(red_channel, client, stream,
> -                 common_caps, caps,
> +    snd_set_peer(red_channel, client, stream, caps,
>                   TYPE_PLAYBACK_CHANNEL_CLIENT);
>  }
>  
> @@ -1283,10 +1281,9 @@ record_channel_client_constructed(GObject
> *object)
>  
>  static void snd_set_record_peer(RedChannel *red_channel, RedClient
> *client, RedsStream *stream,
>                                  G_GNUC_UNUSED int migration,
> -                                GArray *common_caps, GArray *caps)
> +                                RedChannelCapabilities *caps)
>  {
> -    snd_set_peer(red_channel, client, stream,
> -                 common_caps, caps,
> +    snd_set_peer(red_channel, client, stream, caps,
>                   TYPE_RECORD_CHANNEL_CLIENT);
>  }
>  
> diff --git a/server/spicevmc.c b/server/spicevmc.c
> index d35a473..3951fa2 100644
> --- a/server/spicevmc.c
> +++ b/server/spicevmc.c
> @@ -183,7 +183,7 @@ G_DEFINE_TYPE(RedVmcChannelPort,
> red_vmc_channel_port, RED_TYPE_VMC_CHANNEL)
>  
>  static void spicevmc_connect(RedChannel *channel, RedClient *client,
>                               RedsStream *stream, int migration,
> -                             GArray *common_caps, GArray *caps);
> +                             RedChannelCapabilities *caps);
>  
>  static void
>  red_vmc_channel_constructed(GObject *object)
> @@ -765,7 +765,7 @@
> red_vmc_channel_port_class_init(RedVmcChannelPortClass *klass)
>  
>  static void spicevmc_connect(RedChannel *channel, RedClient *client,
>      RedsStream *stream, int migration,
> -    GArray *common_caps, GArray *caps)
> +    RedChannelCapabilities *caps)
>  {
>      RedChannelClient *rcc;
>      RedVmcChannel *vmc_channel;
> @@ -786,8 +786,7 @@ static void spicevmc_connect(RedChannel *channel,
> RedClient *client,
>          return;
>      }
>  
> -    rcc = red_channel_client_create(channel, client, stream, FALSE,
> -                                    common_caps, caps);
> +    rcc = red_channel_client_create(channel, client, stream, FALSE,
> caps);
>      if (!rcc) {
>          return;
>      }


Aside from the comments above, I think the patch is fine. We discussed
this briefly on IRC and talked about the possibility of squashing this
patch with the previous one. I think that's probably a good idea since
this patch actually undoes a bunch of stuff added in the last patch.

Reviewed-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]