Re: [PATCH spice-server v3 5/5] red-channel: Use RedChannelCapabilities directly to pass capabilities

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

 



On Thu, Mar 02, 2017 at 09:16:11AM +0000, Frediano Ziglio wrote:
> For each channel there are two set of capabilities, one
> for the common ones and one for the specific ones.
> A single set were almost always passed using 2 arguments,
> a number of elements and an array but then before using
> these were converted to a GArray.
> Use a single structure (already available) to pass all
> channel capabilites using a single argument.
> 
> Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> ---
>  server/cursor-channel-client.c    | 22 ++------------
>  server/cursor-channel-client.h    |  4 +--
>  server/cursor-channel.c           |  6 ++--
>  server/cursor-channel.h           |  9 +++---
>  server/dcc.c                      | 22 ++------------
>  server/dcc.h                      | 16 ++++------
>  server/inputs-channel-client.c    | 23 ++-------------
>  server/inputs-channel-client.h    |  5 +---
>  server/inputs-channel.c           |  7 ++---
>  server/main-channel-client.c      | 22 ++------------
>  server/main-channel-client.h      |  3 +-
>  server/main-channel.c             |  7 ++---
>  server/main-channel.h             |  4 +--
>  server/red-channel-client.c       | 62 +++++----------------------------------
>  server/red-channel-client.h       |  3 +-
>  server/red-channel.c              | 19 ++++--------
>  server/red-channel.h              |  7 ++---
>  server/red-qxl.c                  | 24 ++++-----------
>  server/red-qxl.h                  | 10 ++-----
>  server/red-worker.c               | 11 +++----
>  server/reds.c                     | 37 +++++++++++++++--------
>  server/smartcard-channel-client.c | 22 ++------------
>  server/smartcard-channel-client.h |  3 +-
>  server/smartcard.c                |  6 ++--
>  server/sound.c                    | 37 +++++------------------
>  server/spicevmc.c                 | 11 ++++---
>  26 files changed, 97 insertions(+), 305 deletions(-)

Nice removal :)

> 
> diff --git a/server/cursor-channel-client.c b/server/cursor-channel-client.c
> index 56efd1e..1a05f73 100644
> --- a/server/cursor-channel-client.c
> +++ b/server/cursor-channel-client.c
> @@ -93,21 +93,9 @@ void cursor_channel_client_migrate(RedChannelClient *rcc)
>  
>  CursorChannelClient* cursor_channel_client_new(CursorChannel *cursor, RedClient *client, RedsStream *stream,
>                                                 int mig_target,
> -                                               uint32_t *common_caps, int num_common_caps,
> -                                               uint32_t *caps, int num_caps)
> +                                               RedChannelCapabilities *caps)
>  {
>      CursorChannelClient *rcc;
> -    GArray *common_caps_array = NULL, *caps_array = NULL;
> -
> -    if (common_caps) {
> -        common_caps_array = g_array_sized_new(FALSE, FALSE, sizeof (*common_caps),
> -                                              num_common_caps);
> -        g_array_append_vals(common_caps_array, common_caps, num_common_caps);
> -    }
> -    if (caps) {
> -        caps_array = g_array_sized_new(FALSE, FALSE, sizeof (*caps), num_caps);
> -        g_array_append_vals(caps_array, caps, num_caps);
> -    }
>  
>      rcc = g_initable_new(TYPE_CURSOR_CHANNEL_CLIENT,
>                           NULL, NULL,
> @@ -115,16 +103,10 @@ CursorChannelClient* cursor_channel_client_new(CursorChannel *cursor, RedClient
>                           "client", client,
>                           "stream", stream,
>                           "monitor-latency", FALSE,
> -                         "common-caps", common_caps_array,
> -                         "caps", caps_array,
> +                         "caps", caps,
>                           NULL);
>      common_graphics_channel_set_during_target_migrate(COMMON_GRAPHICS_CHANNEL(cursor), mig_target);
>  
> -    if (caps_array)
> -        g_array_unref(caps_array);
> -    if (common_caps_array)
> -        g_array_unref(common_caps_array);
> -
>      return rcc;
>  }
>  
> diff --git a/server/cursor-channel-client.h b/server/cursor-channel-client.h
> index d1dd31d..e2aa3a8 100644
> --- a/server/cursor-channel-client.h
> +++ b/server/cursor-channel-client.h
> @@ -63,9 +63,7 @@ CursorChannelClient* cursor_channel_client_new(CursorChannel *cursor,
>                                                 RedClient *client,
>                                                 RedsStream *stream,
>                                                 int mig_target,
> -                                               uint32_t *common_caps,
> -                                               int num_common_caps,
> -                                               uint32_t *caps, int num_caps);
> +                                               RedChannelCapabilities *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 4fe3f8d..5b23a16 100644
> --- a/server/cursor-channel.c
> +++ b/server/cursor-channel.c
> @@ -406,8 +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,
> -                            uint32_t *common_caps, int num_common_caps,
> -                            uint32_t *caps, int num_caps)
> +                            RedChannelCapabilities *caps)
>  {
>      CursorChannelClient *ccc;
>  
> @@ -416,8 +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, num_common_caps,
> -                                    caps, num_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 ec9d44f..abf7b3d 100644
> --- a/server/cursor-channel.h
> +++ b/server/cursor-channel.h
> @@ -72,11 +72,10 @@ void                 cursor_channel_set_mouse_mode(CursorChannel *cursor, uint32
>   * This is the equivalent of RedChannel client connect callback.
>   * See comment on cursor_channel_new.
>   */
> -void                 cursor_channel_connect     (CursorChannel *cursor, RedClient *client,
> -                                                 RedsStream *stream,
> -                                                 int migrate,
> -                                                 uint32_t *common_caps, int num_common_caps,
> -                                                 uint32_t *caps, int num_caps);
> +void cursor_channel_connect(CursorChannel *cursor, RedClient *client,
> +                            RedsStream *stream,
> +                            int migrate,
> +                            RedChannelCapabilities *caps);

I would not reindent this.

>  
>  /**
>   * Migrate a client channel from a CursorChannel.
> diff --git a/server/dcc.c b/server/dcc.c
> index 373dad9..6c1c89e 100644
> --- a/server/dcc.c
> +++ b/server/dcc.c
> @@ -482,25 +482,13 @@ static void dcc_init_stream_agents(DisplayChannelClient *dcc)
>  DisplayChannelClient *dcc_new(DisplayChannel *display,
>                                RedClient *client, RedsStream *stream,
>                                int mig_target,
> -                              uint32_t *common_caps, int num_common_caps,
> -                              uint32_t *caps, int num_caps,
> +                              RedChannelCapabilities *caps,
>                                SpiceImageCompression image_compression,
>                                spice_wan_compression_t jpeg_state,
>                                spice_wan_compression_t zlib_glz_state)
>  
>  {
>      DisplayChannelClient *dcc;
> -    GArray *common_caps_array = NULL, *caps_array = NULL;
> -
> -    if (common_caps) {
> -        common_caps_array = g_array_sized_new(FALSE, FALSE, sizeof (*common_caps),
> -                                              num_common_caps);
> -        g_array_append_vals(common_caps_array, common_caps, num_common_caps);
> -    }
> -    if (caps) {
> -        caps_array = g_array_sized_new(FALSE, FALSE, sizeof (*caps), num_caps);
> -        g_array_append_vals(caps_array, caps, num_caps);
> -    }
>  
>      dcc = g_initable_new(TYPE_DISPLAY_CHANNEL_CLIENT,
>                           NULL, NULL,
> @@ -508,8 +496,7 @@ DisplayChannelClient *dcc_new(DisplayChannel *display,
>                           "client", client,
>                           "stream", stream,
>                           "monitor-latency", TRUE,
> -                         "common-caps", common_caps_array,
> -                         "caps", caps_array,
> +                         "caps", caps,
>                           "image-compression", image_compression,
>                           "jpeg-state", jpeg_state,
>                           "zlib-glz-state", zlib_glz_state,
> @@ -518,11 +505,6 @@ DisplayChannelClient *dcc_new(DisplayChannel *display,
>      common_graphics_channel_set_during_target_migrate(COMMON_GRAPHICS_CHANNEL(display), mig_target);
>      dcc->priv->id = common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(display))->id;
>  
> -    if (common_caps_array)
> -        g_array_unref(common_caps_array);
> -    if (caps_array)
> -        g_array_unref(caps_array);
> -
>      return dcc;
>  }
>  
> diff --git a/server/dcc.h b/server/dcc.h
> index ebdbb8d..9a82a5d 100644
> --- a/server/dcc.h
> +++ b/server/dcc.h
> @@ -130,17 +130,11 @@ typedef struct RedDrawablePipeItem {
>      DisplayChannelClient *dcc;
>  } RedDrawablePipeItem;
>  
> -DisplayChannelClient*      dcc_new                                   (DisplayChannel *display,
> -                                                                      RedClient *client,
> -                                                                      RedsStream *stream,
> -                                                                      int mig_target,
> -                                                                      uint32_t *common_caps,
> -                                                                      int num_common_caps,
> -                                                                      uint32_t *caps,
> -                                                                      int num_caps,
> -                                                                      SpiceImageCompression image_compression,
> -                                                                      spice_wan_compression_t jpeg_state,
> -                                                                      spice_wan_compression_t zlib_glz_state);
> +DisplayChannelClient* dcc_new(DisplayChannel *display, RedClient *client, RedsStream *stream,
> +                              int mig_target, RedChannelCapabilities *caps,
> +                              SpiceImageCompression image_compression,
> +                              spice_wan_compression_t jpeg_state,
> +                              spice_wan_compression_t zlib_glz_state);

Nor this one, this makes the prototypes stand out in the header file.

>  void                       dcc_start                                 (DisplayChannelClient *dcc);
>  void                       dcc_stop                                  (DisplayChannelClient *dcc);
>  int                        dcc_handle_message                        (RedChannelClient *rcc,
> diff --git a/server/inputs-channel-client.c b/server/inputs-channel-client.c
> index 4a534e8..72b5c39 100644
> --- a/server/inputs-channel-client.c
> +++ b/server/inputs-channel-client.c
> @@ -48,38 +48,19 @@ RedChannelClient* inputs_channel_client_create(RedChannel *channel,
>                                                 RedClient *client,
>                                                 RedsStream *stream,
>                                                 int monitor_latency,
> -                                               int num_common_caps,
> -                                               uint32_t *common_caps,
> -                                               int num_caps,
> -                                               uint32_t *caps)
> +                                               RedChannelCapabilities *caps)
>  {
>      RedChannelClient *rcc;
> -    GArray *common_caps_array = NULL, *caps_array = NULL;
>  
> -    if (common_caps) {
> -        common_caps_array = g_array_sized_new(FALSE, FALSE, sizeof (*common_caps),
> -                                              num_common_caps);
> -        g_array_append_vals(common_caps_array, common_caps, num_common_caps);
> -    }
> -    if (caps) {
> -        caps_array = g_array_sized_new(FALSE, FALSE, sizeof (*caps), num_caps);
> -        g_array_append_vals(caps_array, caps, num_caps);
> -    }
>      rcc = g_initable_new(TYPE_INPUTS_CHANNEL_CLIENT,
>                           NULL, NULL,
>                           "channel", channel,
>                           "client", client,
>                           "stream", stream,
>                           "monitor-latency", monitor_latency,
> -                         "caps", caps_array,
> -                         "common-caps", common_caps_array,
> +                         "caps", caps,
>                           NULL);
>  
> -    if (caps_array)
> -        g_array_unref(caps_array);
> -    if (common_caps_array)
> -        g_array_unref(common_caps_array);
> -
>      return rcc;
>  }
>  
> diff --git a/server/inputs-channel-client.h b/server/inputs-channel-client.h
> index 7550b3c..ba08dbf 100644
> --- a/server/inputs-channel-client.h
> +++ b/server/inputs-channel-client.h
> @@ -60,10 +60,7 @@ RedChannelClient* inputs_channel_client_create(RedChannel *channel,
>                                                 RedClient *client,
>                                                 RedsStream *stream,
>                                                 int monitor_latency,
> -                                               int num_common_caps,
> -                                               uint32_t *common_caps,
> -                                               int num_caps,
> -                                               uint32_t *caps);
> +                                               RedChannelCapabilities *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 ed92e71..ec297a2 100644
> --- a/server/inputs-channel.c
> +++ b/server/inputs-channel.c
> @@ -486,8 +486,7 @@ static void inputs_pipe_add_init(RedChannelClient *rcc)
>  
>  static void inputs_connect(RedChannel *channel, RedClient *client,
>                             RedsStream *stream, int migration,
> -                           int num_common_caps, uint32_t *common_caps,
> -                           int num_caps, uint32_t *caps)
> +                           RedChannelCapabilities *caps)
>  {
>      RedChannelClient *rcc;
>  
> @@ -497,9 +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,
> -                                       num_common_caps, common_caps,
> -                                       num_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 6aace29..2b68407 100644
> --- a/server/main-channel-client.c
> +++ b/server/main-channel-client.c
> @@ -651,21 +651,9 @@ static void ping_timer_cb(void *opaque)
>  
>  MainChannelClient *main_channel_client_create(MainChannel *main_chan, RedClient *client,
>                                                RedsStream *stream, uint32_t connection_id,
> -                                              int num_common_caps, uint32_t *common_caps,
> -                                              int num_caps, uint32_t *caps)
> +                                              RedChannelCapabilities *caps)
>  {
>      MainChannelClient *mcc;
> -    GArray *common_caps_array = NULL, *caps_array = NULL;
> -
> -    if (common_caps) {
> -        common_caps_array = g_array_sized_new(FALSE, FALSE, sizeof (*common_caps),
> -                                              num_common_caps);
> -        g_array_append_vals(common_caps_array, common_caps, num_common_caps);
> -    }
> -    if (caps) {
> -        caps_array = g_array_sized_new(FALSE, FALSE, sizeof (*caps), num_caps);
> -        g_array_append_vals(caps_array, caps, num_caps);
> -    }
>  
>      mcc = g_initable_new(TYPE_MAIN_CHANNEL_CLIENT,
>                           NULL, NULL,
> @@ -673,16 +661,10 @@ MainChannelClient *main_channel_client_create(MainChannel *main_chan, RedClient
>                           "client", client,
>                           "stream", stream,
>                           "monitor-latency", FALSE,
> -                         "caps", caps_array,
> -                         "common-caps", common_caps_array,
> +                         "caps", caps,
>                           "connection-id", connection_id,
>                           NULL);
>  
> -    if (caps_array)
> -        g_array_unref(caps_array);
> -    if (common_caps_array)
> -        g_array_unref(common_caps_array);
> -
>      return mcc;
>  }
>  
> diff --git a/server/main-channel-client.h b/server/main-channel-client.h
> index 14fb419..9c7009d 100644
> --- a/server/main-channel-client.h
> +++ b/server/main-channel-client.h
> @@ -58,8 +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,
> -                                              int num_common_caps, uint32_t *common_caps,
> -                                              int num_caps, uint32_t *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 3a6e6cd..307c80f 100644
> --- a/server/main-channel.c
> +++ b/server/main-channel.c
> @@ -287,8 +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,
> -                                     int num_common_caps, uint32_t *common_caps, int num_caps,
> -                                     uint32_t *caps)
> +                                     RedChannelCapabilities *caps)
>  {
>      MainChannelClient *mcc;
>  
> @@ -298,9 +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,
> -                                     num_common_caps, common_caps,
> -                                     num_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 b70649c..0209024 100644
> --- a/server/main-channel.h
> +++ b/server/main-channel.h
> @@ -59,8 +59,8 @@ MainChannel *main_channel_new(RedsState *reds);
>  RedClient *main_channel_get_client_by_link_id(MainChannel *main_chan, uint32_t link_id);
>  /* 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, int num_common_caps,
> -     uint32_t *common_caps, int num_caps, uint32_t *caps);
> +     RedsStream *stream, uint32_t link_id, int migration,
> +     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 2d2e3fd..456d950 100644
> --- a/server/red-channel-client.c
> +++ b/server/red-channel-client.c
> @@ -153,7 +153,6 @@ struct RedChannelClientPrivate
>  static const SpiceDataHeaderOpaque full_header_wrapper;
>  static const SpiceDataHeaderOpaque mini_header_wrapper;
>  static void red_channel_client_clear_sent_item(RedChannelClient *rcc);
> -static void red_channel_client_destroy_remote_caps(RedChannelClient* rcc);
>  static void red_channel_client_initable_interface_init(GInitableIface *iface);
>  static void red_channel_client_set_message_serial(RedChannelClient *channel, uint64_t);
>  
> @@ -188,7 +187,6 @@ enum {
>      PROP_CHANNEL,
>      PROP_CLIENT,
>      PROP_MONITOR_LATENCY,
> -    PROP_COMMON_CAPS,
>      PROP_CAPS
>  };
>  
> @@ -315,25 +313,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_reset(&self->priv->remote_caps);
> +                    red_channel_capabilities_init(&self->priv->remote_caps, caps);

Imo would be easier to just have a RedChannelCapabilities *remote_caps
in RedChannelClientPrivate and to use g_value_dup_boxed(value).

>                  }
>              }
>              break;
> @@ -358,7 +343,7 @@ red_channel_client_finalize(GObject *object)
>          spice_marshaller_destroy(self->priv->send_data.urgent.marshaller);
>      }
>  
> -    red_channel_client_destroy_remote_caps(self);
> +    red_channel_capabilities_reset(&self->priv->remote_caps);
>      if (self->priv->channel) {
>          g_object_unref(self->priv->channel);
>      }
> @@ -434,17 +419,9 @@ 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_WRITABLE
> -                              | 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,

RED_TYPE_CHANNEL_CAPABILITIES, as suggested in the previous patch.

>                                G_PARAM_STATIC_STRINGS
>                                | G_PARAM_WRITABLE
>                                | G_PARAM_CONSTRUCT_ONLY);
> @@ -686,14 +663,6 @@ static gboolean red_channel_client_pipe_remove(RedChannelClient *rcc, RedPipeIte
>      return g_queue_remove(&rcc->priv->pipe, item);
>  }
>  
> -static void red_channel_client_destroy_remote_caps(RedChannelClient* rcc)
> -{
> -    rcc->priv->remote_caps.num_common_caps = 0;
> -    free(rcc->priv->remote_caps.common_caps);
> -    rcc->priv->remote_caps.num_caps = 0;
> -    free(rcc->priv->remote_caps.caps);
> -}
> -
>  int red_channel_client_test_remote_common_cap(RedChannelClient *rcc, uint32_t cap)
>  {
>      return test_capability(rcc->priv->remote_caps.common_caps,
> @@ -997,36 +966,19 @@ cleanup:
>  RedChannelClient *red_channel_client_create(RedChannel *channel, RedClient *client,
>                                              RedsStream *stream,
>                                              int monitor_latency,
> -                                            int num_common_caps, uint32_t *common_caps,
> -                                            int num_caps, uint32_t *caps)
> +                                            RedChannelCapabilities *caps)
>  {
>      RedChannelClient *rcc;
> -    GArray *common_caps_array = NULL, *caps_array = NULL;
>  
> -    if (common_caps) {
> -        common_caps_array = g_array_sized_new(FALSE, FALSE, sizeof (*common_caps),
> -                                              num_common_caps);
> -        g_array_append_vals(common_caps_array, common_caps, num_common_caps);
> -    }
> -    if (caps) {
> -        caps_array = g_array_sized_new(FALSE, FALSE, sizeof (*caps), num_caps);
> -        g_array_append_vals(caps_array, caps, num_caps);
> -    }
>      rcc = g_initable_new(RED_TYPE_CHANNEL_CLIENT,
>                           NULL, NULL,
>                           "channel", channel,
>                           "client", client,
>                           "stream", stream,
>                           "monitor-latency", monitor_latency,
> -                         "caps", caps_array,
> -                         "common-caps", common_caps_array,
> +                         "caps", caps,
>                           NULL);
>  
> -    if (caps_array)
> -        g_array_unref(caps_array);
> -    if (common_caps_array)
> -        g_array_unref(common_caps_array);
> -
>      return rcc;
>  }
>  
> diff --git a/server/red-channel-client.h b/server/red-channel-client.h
> index 474a5cd..1450089 100644
> --- a/server/red-channel-client.h
> +++ b/server/red-channel-client.h
> @@ -50,8 +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,
> -                                            int num_common_caps, uint32_t *common_caps,
> -                                            int num_caps, uint32_t *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 8fe0d33..d78c628 100644
> --- a/server/red-channel.c
> +++ b/server/red-channel.c
> @@ -183,13 +183,7 @@ red_channel_finalize(GObject *object)
>  {
>      RedChannel *self = RED_CHANNEL(object);
>  
> -    if (self->priv->local_caps.num_common_caps) {
> -        free(self->priv->local_caps.common_caps);
> -    }
> -
> -    if (self->priv->local_caps.num_caps) {
> -        free(self->priv->local_caps.caps);
> -    }
> +    red_channel_capabilities_reset(&self->priv->local_caps);
>  
>      G_OBJECT_CLASS(red_channel_parent_class)->finalize(object);
>  }
> @@ -219,8 +213,7 @@ red_channel_constructed(GObject *object)
>  static void red_channel_client_default_connect(RedChannel *channel, RedClient *client,
>                                                 RedsStream *stream,
>                                                 int migration,
> -                                               int num_common_caps, uint32_t *common_caps,
> -                                               int num_caps, uint32_t *caps)
> +                                               RedChannelCapabilities *caps)
>  {
>      spice_error("not implemented");
>  }
> @@ -513,12 +506,10 @@ void red_channel_disconnect(RedChannel *channel)
>  }
>  
>  void red_channel_connect(RedChannel *channel, RedClient *client,
> -                         RedsStream *stream, int migration, int num_common_caps,
> -                         uint32_t *common_caps, int num_caps, uint32_t *caps)
> +                         RedsStream *stream, int migration,
> +                         RedChannelCapabilities *caps)
>  {
> -    channel->priv->client_cbs.connect(channel, client, stream, migration,
> -                                      num_common_caps, common_caps, num_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 6cee35f..e076e2a 100644
> --- a/server/red-channel.h
> +++ b/server/red-channel.h
> @@ -61,8 +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, int num_common_caps, uint32_t *common_caps,
> -                                            int num_caps, uint32_t *caps);
> +                                            int migration, RedChannelCapabilities *caps);
>  typedef void (*channel_client_disconnect_proc)(RedChannelClient *base);
>  typedef void (*channel_client_migrate_proc)(RedChannelClient *base);
>  
> @@ -209,8 +208,8 @@ void red_channel_send(RedChannel *channel);
>  // For red_worker
>  void red_channel_disconnect(RedChannel *channel);
>  void red_channel_connect(RedChannel *channel, RedClient *client,
> -                         RedsStream *stream, int migration, int num_common_caps,
> -                         uint32_t *common_caps, int num_caps, uint32_t *caps);
> +                         RedsStream *stream, int migration,
> +                         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 b6b3770..53f3338 100644
> --- a/server/red-qxl.c
> +++ b/server/red-qxl.c
> @@ -76,8 +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,
> -                                     int num_common_caps, uint32_t *common_caps, int num_caps,
> -                                     uint32_t *caps)
> +                                     RedChannelCapabilities *caps)
>  {
>      RedWorkerMessageDisplayConnect payload = {0,};
>      Dispatcher *dispatcher;
> @@ -87,13 +86,7 @@ static void red_qxl_set_display_peer(RedChannel *channel, RedClient *client,
>      payload.client = client;
>      payload.stream = stream;
>      payload.migration = migration;
> -    payload.num_common_caps = num_common_caps;
> -    payload.common_caps = spice_malloc(sizeof(uint32_t)*num_common_caps);
> -    payload.num_caps = num_caps;
> -    payload.caps = spice_malloc(sizeof(uint32_t)*num_caps);
> -
> -    memcpy(payload.common_caps, common_caps, sizeof(uint32_t)*num_common_caps);
> -    memcpy(payload.caps, caps, sizeof(uint32_t)*num_caps);
> +    red_channel_capabilities_init(&payload.caps, caps);
>  
>      dispatcher_send_message(dispatcher,
>                              RED_WORKER_MESSAGE_DISPLAY_CONNECT,
> @@ -142,9 +135,8 @@ static void red_qxl_display_migrate(RedChannelClient *rcc)
>  }
>  
>  static void red_qxl_set_cursor_peer(RedChannel *channel, RedClient *client, RedsStream *stream,
> -                                    int migration, int num_common_caps,
> -                                    uint32_t *common_caps, int num_caps,
> -                                    uint32_t *caps)
> +                                    int migration,
> +                                    RedChannelCapabilities *caps)
>  {
>      RedWorkerMessageCursorConnect payload = {0,};
>      Dispatcher *dispatcher = (Dispatcher *)g_object_get_data(G_OBJECT(channel), "dispatcher");
> @@ -152,13 +144,7 @@ static void red_qxl_set_cursor_peer(RedChannel *channel, RedClient *client, Reds
>      payload.client = client;
>      payload.stream = stream;
>      payload.migration = migration;
> -    payload.num_common_caps = num_common_caps;
> -    payload.common_caps = spice_malloc(sizeof(uint32_t)*num_common_caps);
> -    payload.num_caps = num_caps;
> -    payload.caps = spice_malloc(sizeof(uint32_t)*num_caps);
> -
> -    memcpy(payload.common_caps, common_caps, sizeof(uint32_t)*num_common_caps);
> -    memcpy(payload.caps, caps, sizeof(uint32_t)*num_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 0c9498a..ca926de 100644
> --- a/server/red-qxl.h
> +++ b/server/red-qxl.h
> @@ -126,11 +126,8 @@ enum {
>  typedef struct RedWorkerMessageDisplayConnect {
>      RedClient * client;
>      RedsStream * stream;
> -    uint32_t *common_caps; // red_worker should free
> -    uint32_t *caps;        // red_worker should free
> +    RedChannelCapabilities caps;        // red_worker should destroy

'destroy' is no longer accurate.

>      int migration;
> -    int num_common_caps;
> -    int num_caps;
>  } RedWorkerMessageDisplayConnect;
>  
>  typedef struct RedWorkerMessageDisplayDisconnect {
> @@ -145,10 +142,7 @@ typedef struct RedWorkerMessageCursorConnect {
>      RedClient *client;
>      RedsStream *stream;
>      int migration;
> -    uint32_t *common_caps; // red_worker should free
> -    int num_common_caps;
> -    uint32_t *caps;        // red_worker should free
> -    int num_caps;
> +    RedChannelCapabilities caps;        // red_worker should destroy

Ditto.

>  } RedWorkerMessageCursorConnect;
>  
>  typedef struct RedWorkerMessageCursorDisconnect {
> diff --git a/server/red-worker.c b/server/red-worker.c
> index e5adbaa..831546e 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->num_common_caps, msg->caps, msg->num_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);
>  
> -    free(msg->caps);
> -    free(msg->common_caps);
> +    red_channel_capabilities_reset(&msg->caps);
>  }
>  
>  static void handle_dev_display_disconnect(void *opaque, void *payload)
> @@ -832,10 +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->num_common_caps,
> -                           msg->caps, msg->num_caps);
> -    free(msg->caps);
> -    free(msg->common_caps);
> +                           &msg->caps);
> +    red_channel_capabilities_reset(&msg->caps);
>  }
>  
>  static void handle_dev_cursor_disconnect(void *opaque, void *payload)
> diff --git a/server/reds.c b/server/reds.c
> index a28653c..68f32c9 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -1675,6 +1675,22 @@ static RedClient *reds_get_client(RedsState *reds)
>      return reds->clients->data;
>  }
>  
> +static void
> +red_channel_capabilities_from_link(RedChannelCapabilities *caps, const SpiceLinkMess *link_mess)

_from_link_message() ?

> +{
> +    uint32_t *raw_caps = (uint32_t *)((uint8_t *)link_mess + link_mess->caps_offset);
> +
> +    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;

Let's use 'if' instead of ? here, this is not really readable this way
with all the line splitting.


Overall, looks good, thanks!

Christophe

Attachment: signature.asc
Description: PGP signature

_______________________________________________
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]