[Spice-devel] [PATCH] Store 'renderers' as GArray in RedsState

[Frediano Ziglio]

> Subject: [PATCH] Store 'renderers' as GArray in RedsState
> 

This version change just that renderers are not stored in DisplayChannel.
Note that this (as your patch) share the array content causing theoretical
problems if the array is changed in Reds while accessed by DisplayChannel.

Frediano

> ---
>  server/display-channel.c | 12 +++++-------
>  server/display-channel.h |  2 --
>  server/reds-private.h    |  1 +
>  server/reds.c            | 22 +++++++++++++---------
>  server/reds.h            |  5 ++---
>  5 files changed, 21 insertions(+), 21 deletions(-)
> 
> diff --git a/server/display-channel.c b/server/display-channel.c
> index b4f7ede..78c984f 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -1911,10 +1911,12 @@ void display_channel_create_surface(DisplayChannel
> *display, uint32_t surface_id
>  
>      if (display->renderer == RED_RENDERER_INVALID) {
>          int i;
> -        for (i = 0; i < display->num_renderers; i++) {
> -            surface->context.canvas = create_canvas_for_surface(display,
> surface, display->renderers[i]);
> +        GArray *renderers = reds_get_renderers(reds);
> +        for (i = 0; i < renderers->len; i++) {
> +            uint32_t renderer = g_array_index(renderers, uint32_t, i);
> +            surface->context.canvas = create_canvas_for_surface(display,
> surface, renderer);
>              if (surface->context.canvas) {
> -                display->renderer = display->renderers[i];
> +                display->renderer = renderer;
>                  break;
>              }
>          }
> @@ -2031,8 +2033,6 @@ DisplayChannel* display_channel_new(RedWorker *worker,
> int migrate, int stream_v
>          image_surfaces_get,
>      };
>  
> -    spice_return_val_if_fail(num_renderers > 0, NULL);
> -
>      spice_info("create display channel");
>      display = (DisplayChannel *)red_worker_new_channel(
>          worker, sizeof(*display), "display_channel",
> @@ -2063,8 +2063,6 @@ DisplayChannel* display_channel_new(RedWorker *worker,
> int migrate, int stream_v
>      stat_compress_init(&display->lz4_stat, "lz4", stat_clock);
>  
>      display->n_surfaces = n_surfaces;
> -    display->num_renderers = num_renderers;
> -    memcpy(display->renderers, renderers, sizeof(display->renderers));
>      display->renderer = RED_RENDERER_INVALID;
>  
>      ring_init(&display->current_list);
> diff --git a/server/display-channel.h b/server/display-channel.h
> index 82fd663..e0b18ca 100644
> --- a/server/display-channel.h
> +++ b/server/display-channel.h
> @@ -170,8 +170,6 @@ struct DisplayChannel {
>  
>      MonitorsConfig *monitors_config;
>  
> -    uint32_t num_renderers;
> -    uint32_t renderers[RED_RENDERER_LAST];
>      uint32_t renderer;
>      int enable_jpeg;
>      int enable_zlib_glz_wrap;
> diff --git a/server/reds-private.h b/server/reds-private.h
> index 21dbe49..e5444d6 100644
> --- a/server/reds-private.h
> +++ b/server/reds-private.h
> @@ -204,6 +204,7 @@ struct RedsState {
>  
>      int default_channel_security;
>      ChannelSecurityOptions *channels_security;
> +    GArray *renderers;
>  
>      int spice_port;
>      int spice_secure_port;
> diff --git a/server/reds.c b/server/reds.c
> index 58af7c4..d1527a9 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -3403,6 +3403,7 @@ SPICE_GNUC_VISIBLE SpiceServer *spice_server_new(void)
>      reds = spice_new0(RedsState, 1);
>      reds->default_channel_security =
>          SPICE_CHANNEL_SECURITY_NONE | SPICE_CHANNEL_SECURITY_SSL;
> +    reds->renderers = g_array_sized_new(FALSE, TRUE, sizeof(uint32_t),
> RED_RENDERER_LAST);
>      reds->spice_port = -1;
>      reds->spice_secure_port = -1;
>      reds->spice_listen_socket_fd = -1;
> @@ -3435,9 +3436,6 @@ static const RendererInfo renderers_info[] = {
>      {RED_RENDERER_INVALID, NULL},
>  };
>  
> -uint32_t renderers[RED_RENDERER_LAST];
> -uint32_t num_renderers = 0;
> -
>  static const RendererInfo *find_renderer(const char *name)
>  {
>      const RendererInfo *inf = renderers_info;
> @@ -3450,14 +3448,14 @@ static const RendererInfo *find_renderer(const char
> *name)
>      return NULL;
>  }
>  
> -static int red_add_renderer(const char *name)
> +static int reds_add_renderer(RedsState *reds, const char *name)
>  {
>      const RendererInfo *inf;
>  
> -    if (num_renderers == RED_RENDERER_LAST || !(inf = find_renderer(name)))
> {
> +    if (reds->renderers->len == RED_RENDERER_LAST || !(inf =
> find_renderer(name))) {
>          return FALSE;
>      }
> -    renderers[num_renderers++] = inf->id;
> +    g_array_append_val(reds->renderers, inf->id);
>      return TRUE;
>  }
>  
> @@ -3467,8 +3465,8 @@ SPICE_GNUC_VISIBLE int spice_server_init(SpiceServer
> *s, SpiceCoreInterface *cor
>  
>      spice_assert(reds == s);
>      ret = do_spice_init(s, core);
> -    if (num_renderers == 0) {
> -        red_add_renderer(default_renderer);
> +    if (s->renderers->len == 0) {
> +        reds_add_renderer(s, default_renderer);
>      }
>      return ret;
>  }
> @@ -3476,6 +3474,7 @@ SPICE_GNUC_VISIBLE int spice_server_init(SpiceServer
> *s, SpiceCoreInterface *cor
>  SPICE_GNUC_VISIBLE void spice_server_destroy(SpiceServer *s)
>  {
>      spice_assert(reds == s);
> +    g_array_unref(s->renderers);
>      reds_exit();
>  }
>  
> @@ -3761,7 +3760,7 @@ SPICE_GNUC_VISIBLE int
> spice_server_is_server_mouse(SpiceServer *s)
>  SPICE_GNUC_VISIBLE int spice_server_add_renderer(SpiceServer *s, const char
>  *name)
>  {
>      spice_assert(reds == s);
> -    if (!red_add_renderer(name)) {
> +    if (!reds_add_renderer(s, name)) {
>          return -1;
>      }
>      return 0;
> @@ -4014,6 +4013,11 @@ SPICE_GNUC_VISIBLE void
> spice_server_set_keepalive_timeout(SpiceServer *s, int t
>      spice_debug("keepalive timeout=%d", timeout);
>  }
>  
> +GArray* reds_get_renderers(RedsState *reds)
> +{
> +    return reds->renderers;
> +}
> +
>  spice_wan_compression_t reds_get_jpeg_state(const RedsState *reds)
>  {
>      return reds->jpeg_state;
> diff --git a/server/reds.h b/server/reds.h
> index d1a726f..8962d78 100644
> --- a/server/reds.h
> +++ b/server/reds.h
> @@ -66,6 +66,8 @@ gboolean reds_get_agent_mouse(const RedsState *reds); //
> used by inputs_channel
>  int reds_has_vdagent(RedsState *reds); // used by inputs channel
>  void reds_handle_agent_mouse_event(RedsState *reds, const VDAgentMouseState
>  *mouse_state); // used by inputs_channel
>  
> +GArray* reds_get_renderers(RedsState *reds);
> +
>  enum {
>      RED_RENDERER_INVALID,
>      RED_RENDERER_SW,
> @@ -73,9 +75,6 @@ enum {
>      RED_RENDERER_LAST
>  };
>  
> -extern uint32_t renderers[RED_RENDERER_LAST];
> -extern uint32_t num_renderers;
> -
>  // Temporary measures to make splitting reds.c to inputs-channel.c easier
>  
>  /* should be called only from main_dispatcher */
> --
> 2.5.0
> 
>