> 
> This improves the readability of the code and keeps things
> encapsulated better.
> ---
> Changes in v2:
>  - changed some loops from while to for
>  - moved some declarations within loop scope
> 
>  server/dcc.c             | 12 +++++-------
>  server/dcc.h             |  1 -
>  server/display-channel.c | 34 +++++++++++++++++++++-------------
>  server/display-channel.h |  6 +-----
>  server/stream.c          |  8 ++++++--
>  5 files changed, 33 insertions(+), 28 deletions(-)
> 
> diff --git a/server/dcc.c b/server/dcc.c
> index ddc1a59..922fbef 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;
>          }
> @@ -286,9 +287,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);
>  }
> @@ -301,8 +300,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 7f93186..9dac8a3 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 e37b5ae..426b9c0 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,14 +265,17 @@ 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;
> +
> +    for (l = pos_after->pipes; l != NULL; l = 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);
>      }
> +
>      if (num_other_linked == 0) {
>          pipes_add_drawable(display, drawable);
>          return;
> @@ -282,12 +285,15 @@ 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;
> +            for (l = pos_after->pipes; l != NULL; l = l->next) {
> +                dpi_pos_after = l->data;
>                  if (dpi_pos_after->dcc == dcc) {
>                      sent = 1;
>                      break;
>                  }
>              }
> +
>              if (!sent) {
>                  dcc_prepend_drawable(dcc, drawable);
>              }
> @@ -324,14 +330,17 @@ static void current_remove_drawable(DisplayChannel
> *display, Drawable *item)
>  static void drawable_remove_from_pipes(Drawable *drawable)
>  {
>      RedDrawablePipeItem *dpi;
> -    RingItem *item, *next;
> +    GList *l;
>  
> -    RING_FOREACH_SAFE(item, next, &drawable->pipes) {
> +    l = drawable->pipes;
> +    while (l) {
> +        GList *next = l->next;
>          RedChannelClient *rcc;
>  
> -        dpi = SPICE_UPCAST(RedDrawablePipeItem, item);
> +        dpi = l->data;
>          rcc = RED_CHANNEL_CLIENT(dpi->dcc);
>          red_channel_client_pipe_remove_and_release(rcc,
>          &dpi->dpi_pipe_item);
> +        l = next;
>      }
>  }
>  
> @@ -424,7 +433,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++;
> @@ -432,11 +441,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);
>                  }
> @@ -1292,7 +1301,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;
>  
> @@ -1343,7 +1351,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 f559a10..0f2aa3f 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 9517fac..f17a08b 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;
>      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) {
> +        GList *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;
>      }
>  
>  

This last loop causes an possible infinite loop due to a continue in
the loop.
I'm still convinced the old macro were better although now with
all GList*/void* it's hard to make them safe like they were.
I'll try to come out with some proposal.

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

Reply via email to