[Spice-devel] [PATCH 07/10] Change Drawable->pipes from Ring to GList

Christophe Fergeau cfergeau at redhat.com
Fri Sep 9 11:45:15 UTC 2016


On Thu, Sep 08, 2016 at 11:52:57AM -0500, Jonathon Jongsma wrote:
> This greatly improves the readability of the code and keeps things
> encapsulated better.
> ---
>  server/dcc.c             | 12 +++++-------
>  server/dcc.h             |  1 -
>  server/display-channel.c | 39 ++++++++++++++++++++++++++-------------
>  server/display-channel.h |  6 +-----
>  server/stream.c          |  8 ++++++--
>  5 files changed, 38 insertions(+), 28 deletions(-)
> 
> diff --git a/server/dcc.c b/server/dcc.c
> index b12d034..d8cdc45 100644
> --- a/server/dcc.c
> +++ b/server/dcc.c
> @@ -50,9 +50,10 @@ static RedSurfaceCreateItem *red_surface_create_item_new(RedChannel* channel,
>  int dcc_drawable_is_in_pipe(DisplayChannelClient *dcc, Drawable *drawable)
>  {
>      RedDrawablePipeItem *dpi;
> -    RingItem *dpi_link, *dpi_next;
> +    GList *l;
>  
> -    DRAWABLE_FOREACH_DPI_SAFE(drawable, dpi_link, dpi_next, dpi) {
> +    for (l = drawable->pipes; l != NULL; l = l->next) {
> +        dpi = l->data;
>          if (dpi->dcc == dcc) {
>              return TRUE;
>          }
> @@ -288,9 +289,7 @@ static void red_drawable_pipe_item_free(RedPipeItem *item)
>  
>      spice_warn_if_fail(!red_channel_client_pipe_item_is_linked(RED_CHANNEL_CLIENT(dpi->dcc),
>                                                                 &dpi->dpi_pipe_item));
> -    if (ring_item_is_linked(&dpi->base)) {
> -        ring_remove(&dpi->base);
> -    }
> +    dpi->drawable->pipes = g_list_remove(dpi->drawable->pipes, dpi);
>      drawable_unref(dpi->drawable);
>      free(dpi);
>  }
> @@ -303,8 +302,7 @@ static RedDrawablePipeItem *red_drawable_pipe_item_new(DisplayChannelClient *dcc
>      dpi = spice_malloc0(sizeof(*dpi));
>      dpi->drawable = drawable;
>      dpi->dcc = dcc;
> -    ring_item_init(&dpi->base);
> -    ring_add(&drawable->pipes, &dpi->base);
> +    drawable->pipes = g_list_prepend(drawable->pipes, dpi);
>      red_pipe_item_init_full(&dpi->dpi_pipe_item, RED_PIPE_ITEM_TYPE_DRAW,
>                              red_drawable_pipe_item_free);
>      drawable->refs++;
> diff --git a/server/dcc.h b/server/dcc.h
> index d08e413..8237282 100644
> --- a/server/dcc.h
> +++ b/server/dcc.h
> @@ -91,7 +91,6 @@ typedef struct RedImageItem {
>  } RedImageItem;
>  
>  typedef struct RedDrawablePipeItem {
> -    RingItem base;  /* link for a list of pipe items held by Drawable */
>      RedPipeItem dpi_pipe_item; /* link for the client's pipe itself */
>      Drawable *drawable;
>      DisplayChannelClient *dcc;
> diff --git a/server/display-channel.c b/server/display-channel.c
> index bd2907f..b20b330 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -255,7 +255,7 @@ static void pipes_add_drawable(DisplayChannel *display, Drawable *drawable)
>      DisplayChannelClient *dcc;
>      GList *link, *next;
>  
> -    spice_warn_if_fail(ring_is_empty(&drawable->pipes));
> +    spice_warn_if_fail(drawable->pipes == NULL);
>      FOREACH_CLIENT(display, link, next, dcc) {
>          dcc_prepend_drawable(dcc, drawable);
>      }
> @@ -265,13 +265,19 @@ static void pipes_add_drawable_after(DisplayChannel *display,
>                                       Drawable *drawable, Drawable *pos_after)
>  {
>      RedDrawablePipeItem *dpi_pos_after;
> -    RingItem *dpi_link, *dpi_next;
>      DisplayChannelClient *dcc;
>      int num_other_linked = 0;
> +    GList *l, *next;
> +
> +    l = pos_after->pipes;
> +    while (l) {
> +        next = l->next;
> +        dpi_pos_after = l->data;
>  
> -    DRAWABLE_FOREACH_DPI_SAFE(pos_after, dpi_link, dpi_next, dpi_pos_after) {
>          num_other_linked++;
>          dcc_add_drawable_after(dpi_pos_after->dcc, drawable, &dpi_pos_after->dpi_pipe_item);
> +
> +        l = next;
>      }

I'd use a regular for () loop here, 'next' does not seem to be needed.

>      if (num_other_linked == 0) {
>          pipes_add_drawable(display, drawable);
> @@ -282,11 +288,16 @@ static void pipes_add_drawable_after(DisplayChannel *display,
>          spice_debug("TODO: not O(n^2)");
>          FOREACH_CLIENT(display, link, next, dcc) {
>              int sent = 0;
> -            DRAWABLE_FOREACH_DPI_SAFE(pos_after, dpi_link, dpi_next, dpi_pos_after) {
> +            GList *l, *next;
> +            l = pos_after->pipes;
> +            while (l) {

It seems this can also be a regular for() loop.

> +                next = l->next;
> +                dpi_pos_after = l->data;
>                  if (dpi_pos_after->dcc == dcc) {
>                      sent = 1;
>                      break;
>                  }
> +                l = next;
>              }
>              if (!sent) {
>                  dcc_prepend_drawable(dcc, drawable);
> @@ -324,17 +335,20 @@ static void current_remove_drawable(DisplayChannel *display, Drawable *item)
>  static void drawable_remove_from_pipes(Drawable *drawable)
>  {
>      RedDrawablePipeItem *dpi;
> -    RingItem *item, *next;
> +    GList *l, *next;
>  
> -    RING_FOREACH_SAFE(item, next, &drawable->pipes) {
> +    l = drawable->pipes;
> +    while (l) {

I'd declare 'next' here.

> +        next = l->next;
>          RedChannelClient *rcc;
>  
> -        dpi = SPICE_UPCAST(RedDrawablePipeItem, item);
> +        dpi = l->data;
>          rcc = RED_CHANNEL_CLIENT(dpi->dcc);
>          if (red_channel_client_pipe_item_is_linked(rcc, &dpi->dpi_pipe_item)) {
>              red_channel_client_pipe_remove_and_release(rcc,
>                                                         &dpi->dpi_pipe_item);
>          }
> +        l = next;
>      }
>  }
>  
> @@ -427,7 +441,7 @@ static int current_add_equal(DisplayChannel *display, DrawItem *item, TreeItem *
>          if (is_same_drawable(drawable, other_drawable)) {
>  
>              DisplayChannelClient *dcc;
> -            RingItem *dpi_ring_item;
> +            GList *dpi_item;
>              GList *link, *next;
>  
>              other_drawable->refs++;
> @@ -435,11 +449,11 @@ static int current_add_equal(DisplayChannel *display, DrawItem *item, TreeItem *
>  
>              /* sending the drawable to clients that already received
>               * (or will receive) other_drawable */
> -            dpi_ring_item = ring_get_head(&other_drawable->pipes);
> +            dpi_item = g_list_first(other_drawable->pipes);
>              /* dpi contains a sublist of dcc's, ordered the same */
>              FOREACH_CLIENT(display, link, next, dcc) {
> -                if (dpi_ring_item && dcc == SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item)->dcc) {
> -                    dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
> +                if (dpi_item && dcc == ((RedDrawablePipeItem *) dpi_item->data)->dcc) {
> +                    dpi_item = dpi_item->next;
>                  } else {
>                      dcc_prepend_drawable(dcc, drawable);
>                  }
> @@ -1295,7 +1309,6 @@ static Drawable *display_channel_drawable_try_new(DisplayChannel *display,
>      ring_item_init(&drawable->tree_item.base.siblings_link);
>      drawable->tree_item.base.type = TREE_ITEM_TYPE_DRAWABLE;
>      region_init(&drawable->tree_item.base.rgn);
> -    ring_init(&drawable->pipes);
>      glz_retention_init(&drawable->glz_retention);
>      drawable->process_commands_generation = process_commands_generation;
>  
> @@ -1346,7 +1359,7 @@ void drawable_unref(Drawable *drawable)
>          return;
>  
>      spice_warn_if_fail(!drawable->tree_item.shadow);
> -    spice_warn_if_fail(ring_is_empty(&drawable->pipes));
> +    spice_warn_if_fail(drawable->pipes == NULL);
>  
>      if (drawable->stream) {
>          detach_stream(display, drawable->stream);
> diff --git a/server/display-channel.h b/server/display-channel.h
> index 06d30d8..a4ae847 100644
> --- a/server/display-channel.h
> +++ b/server/display-channel.h
> @@ -54,7 +54,7 @@ struct Drawable {
>      RingItem surface_list_link;
>      RingItem list_link;
>      DrawItem tree_item;
> -    Ring pipes;
> +    GList *pipes;
>      RedPipeItem *pipe_item_rest;
>      uint32_t size_pipe_item_rest;
>      RedDrawable *red_drawable;
> @@ -80,10 +80,6 @@ struct Drawable {
>  
>  void drawable_unref (Drawable *drawable);
>  
> -#define LINK_TO_DPI(ptr) SPICE_UPCAST(RedDrawablePipeItem, (ptr))
> -#define DRAWABLE_FOREACH_DPI_SAFE(drawable, link, next, dpi)            \
> -    SAFE_FOREACH(link, next, drawable,  &(drawable)->pipes, dpi, LINK_TO_DPI(link))
> -
>  enum {
>      RED_PIPE_ITEM_TYPE_DRAW = RED_PIPE_ITEM_TYPE_COMMON_LAST,
>      RED_PIPE_ITEM_TYPE_IMAGE,
> diff --git a/server/stream.c b/server/stream.c
> index fef9dc5..089b27f 100644
> --- a/server/stream.c
> +++ b/server/stream.c
> @@ -336,7 +336,7 @@ static void before_reattach_stream(DisplayChannel *display,
>      DisplayChannelClient *dcc;
>      int index;
>      StreamAgent *agent;
> -    RingItem *ring_item, *next;
> +    GList *dpi_item, *dpi_next;
>      GList *link, *link_next;
>  
>      spice_return_if_fail(stream->current);
> @@ -351,7 +351,10 @@ static void before_reattach_stream(DisplayChannel *display,
>      }
>  
>      index = display_channel_get_stream_id(display, stream);
> -    DRAWABLE_FOREACH_DPI_SAFE(stream->current, ring_item, next, dpi) {
> +    dpi_item = stream->current->pipes;
> +    while (dpi_item) {

Declare dpi_next here.

> +        dpi_next = dpi_item->next;
> +        dpi = dpi_item->data;
>          dcc = dpi->dcc;
>          agent = dcc_get_stream_agent(dcc, index);
>  
> @@ -371,6 +374,7 @@ static void before_reattach_stream(DisplayChannel *display,
>                  ++agent->drops;
>              }
>          }
> +        dpi_item = dpi_next;
>      }
>  
>  
> -- 
> 2.7.4
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160909/e1180b25/attachment.sig>


More information about the Spice-devel mailing list