[Spice-devel] [PATCH v2 6/9] Change Drawable->pipes from Ring to GList
Frediano Ziglio
fziglio at redhat.com
Fri Sep 16 08:04:14 UTC 2016
>
> 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
More information about the Spice-devel
mailing list