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

Frediano Ziglio fziglio at redhat.com
Fri Jan 29 02:43:48 PST 2016


> 
> On Wed, 2016-01-27 at 12:48 +0000, Frediano Ziglio wrote:
> > From: Jonathon Jongsma <jjongsma at redhat.com>
> > 
> > Acked-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> >  server/display-channel.c | 13 +++++++------
> >  server/display-channel.h |  3 +--
> >  server/reds-private.h    |  2 ++
> >  server/reds.c            | 20 ++++++++++++--------
> >  server/reds.h            |  5 ++---
> >  5 files changed, 24 insertions(+), 19 deletions(-)
> > 
> > diff --git a/server/display-channel.c b/server/display-channel.c
> > index f0d133a..2282847 100644
> > --- a/server/display-channel.c
> > +++ b/server/display-channel.c
> > @@ -1911,10 +1911,11 @@ 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]);
> > +        for (i = 0; i < display->renderers->len; i++) {
> > +            uint32_t renderer = g_array_index(display->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;
> >              }
> >          }
> > @@ -2030,8 +2031,9 @@ DisplayChannel* display_channel_new(RedWorker
> > *worker, int migrate, int stream_v
> >      static SpiceImageSurfacesOps image_surfaces_ops = {
> >          image_surfaces_get,
> >      };
> > +    GArray *renderers = reds_get_renderers(reds);
> >  
> > -    spice_return_val_if_fail(num_renderers > 0, NULL);
> > +    spice_return_val_if_fail(renderers->len > 0, NULL);
> >  
> >      spice_info("create display channel");
> >      display = (DisplayChannel *)red_worker_new_channel(
> > @@ -2063,8 +2065,7 @@ 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->renderers = g_array_ref(renderers);
> 
> and no g_array_unref(). Add TODO or FIXME ?
> 
> Pavel
> 

actually this g_array_ref is not needed, we already own it, we are
just transferring the ownership.
About the unref as most of the big stuff in red worker there is
no clean up, memory is freed by the OS when program exit.
Adding a clean up is in my list of TODOs.

Frediano

> >      display->renderer = RED_RENDERER_INVALID;
> >  
> >      ring_init(&display->current_list);
> > diff --git a/server/display-channel.h b/server/display-channel.h
> > index bf29cd3..34fb57f 100644
> > --- a/server/display-channel.h
> > +++ b/server/display-channel.h
> > @@ -168,8 +168,7 @@ struct DisplayChannel {
> >  
> >      MonitorsConfig *monitors_config;
> >  
> > -    uint32_t num_renderers;
> > -    uint32_t renderers[RED_RENDERER_LAST];
> > +    GArray *renderers;
> >      uint32_t renderer;
> >      int enable_jpeg;
> >      int enable_zlib_glz_wrap;
> > diff --git a/server/reds-private.h b/server/reds-private.h
> > index 75a4f59..7a3b524 100644
> > --- a/server/reds-private.h
> > +++ b/server/reds-private.h
> > @@ -196,6 +196,8 @@ struct RedsState {
> >      int default_channel_security;
> >      ChannelSecurityOptions *channels_security;
> >      const char *default_renderer;
> > +    GArray *renderers;
> > +
> >  };
> >  
> >  #endif
> > diff --git a/server/reds.c b/server/reds.c
> > index 3f2a132..c671e7a 100644
> > --- a/server/reds.c
> > +++ b/server/reds.c
> > @@ -3446,6 +3446,7 @@ SPICE_GNUC_VISIBLE SpiceServer
> > *spice_server_new(void)
> >      reds->default_channel_security =
> >          SPICE_CHANNEL_SECURITY_NONE | SPICE_CHANNEL_SECURITY_SSL;
> >      reds->default_renderer = "sw";
> > +    reds->renderers = g_array_sized_new(FALSE, TRUE,
> > sizeof(uint32_t), RED_RENDERER_LAST);
> >      return reds;
> >  }
> >  
> > @@ -3459,9 +3460,6 @@ static RendererInfo renderers_info[] = {
> >      {RED_RENDERER_INVALID, NULL},
> >  };
> >  
> > -uint32_t renderers[RED_RENDERER_LAST];
> > -uint32_t num_renderers = 0;
> > -
> >  static RendererInfo *find_renderer(const char *name)
> >  {
> >      RendererInfo *inf = renderers_info;
> > @@ -3474,14 +3472,14 @@ static 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)
> >  {
> >      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;
> >  }
> >  
> > @@ -3492,7 +3490,7 @@ SPICE_GNUC_VISIBLE int
> > spice_server_init(SpiceServer *s, SpiceCoreInterface *cor
> >      spice_assert(reds == s);
> >      ret = do_spice_init(s, core);
> >      if (s->default_renderer) {
> > -        red_add_renderer(s->default_renderer);
> > +        reds_add_renderer(s, s->default_renderer);
> >      }
> >      return ret;
> >  }
> > @@ -3500,6 +3498,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();
> >  }
> >  
> > @@ -3783,7 +3782,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;
> >      }
> >      s->default_renderer = NULL;
> > @@ -4031,3 +4030,8 @@ SPICE_GNUC_VISIBLE void
> > spice_server_set_keepalive_timeout(SpiceServer *s, int t
> >      reds->keepalive_timeout = timeout;
> >      spice_debug("keepalive timeout=%d", timeout);
> >  }
> > +
> > +GArray* reds_get_renderers(RedsState *reds)
> > +{
> > +    return reds->renderers;
> > +}
> > diff --git a/server/reds.h b/server/reds.h
> > index e398607..4034199 100644
> > --- a/server/reds.h
> > +++ b/server/reds.h
> > @@ -63,6 +63,8 @@ int reds_get_agent_mouse(void); // used by
> > inputs_channel
> >  int reds_has_vdagent(void); // 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,
> > @@ -70,9 +72,6 @@ enum {
> >      RED_RENDERER_LAST
> >  };
> >  
> > -extern uint32_t renderers[RED_RENDERER_LAST];
> > -extern uint32_t num_renderers;
> > -
> >  extern struct SpiceCoreInterfaceInternal *core;
> >  extern uint32_t streaming_video;
> >  extern SpiceImageCompression image_compression;
> 


More information about the Spice-devel mailing list