[Spice-devel] [PATCH 4/7] Replace RedChannel::clients with GList

Frediano Ziglio fziglio at redhat.com
Sun May 22 08:02:11 UTC 2016


> 
> Instead of using a Ring, use a GList to store the list of channel
> clients. This allows us to iterate the clients without poking inside of
> the client struct to get the channel_link. This is required in order to
> make the RedChannelClient struct private.
> ---
>  server/display-channel.c |  64 ++++++++--------
>  server/display-channel.h |  16 ++--
>  server/main-channel.c    |  42 +++++------
>  server/red-channel.c     | 193
>  ++++++++++++++++++++---------------------------
>  server/red-channel.h     |   3 +-
>  server/red-worker.c      |   6 +-
>  server/red-worker.h      |  15 ++--
>  server/stream.c          |  23 +++---
>  8 files changed, 162 insertions(+), 200 deletions(-)
> 
> diff --git a/server/display-channel.c b/server/display-channel.c
> index 3f414fd..c94bc26 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -221,7 +221,7 @@ void display_channel_surface_unref(DisplayChannel
> *display, uint32_t surface_id)
>      RedSurface *surface = &display->surfaces[surface_id];
>      QXLInstance *qxl = display->common.qxl;
>      DisplayChannelClient *dcc;
> -    RingItem *link, *next;
> +    GList *link, *next;
>  
>      if (--surface->refs != 0) {
>          return;
> @@ -254,7 +254,7 @@ static void streams_update_visible_region(DisplayChannel
> *display, Drawable *dra
>  {
>      Ring *ring;
>      RingItem *item;
> -    RingItem *dcc_ring_item, *next;
> +    GList *link, *next;
>      DisplayChannelClient *dcc;
>  
>      if (!red_channel_is_connected(RED_CHANNEL(display))) {
> @@ -278,7 +278,7 @@ static void streams_update_visible_region(DisplayChannel
> *display, Drawable *dra
>              continue;
>          }
>  
> -        FOREACH_DCC(display, dcc_ring_item, next, dcc) {
> +        FOREACH_DCC(display, link, next, dcc) {
>              agent = &dcc->stream_agents[get_stream_id(display, stream)];
>  
>              if (region_intersects(&agent->vis_region,
>              &drawable->tree_item.base.rgn)) {
> @@ -293,10 +293,10 @@ static void
> streams_update_visible_region(DisplayChannel *display, Drawable *dra
>  static void pipes_add_drawable(DisplayChannel *display, Drawable *drawable)
>  {
>      DisplayChannelClient *dcc;
> -    RingItem *dcc_ring_item, *next;
> +    GList *link, *next;
>  
>      spice_warn_if_fail(ring_is_empty(&drawable->pipes));
> -    FOREACH_DCC(display, dcc_ring_item, next, dcc) {
> +    FOREACH_DCC(display, link, next, dcc) {
>          dcc_prepend_drawable(dcc, drawable);
>      }
>  }
> @@ -318,9 +318,9 @@ static void pipes_add_drawable_after(DisplayChannel
> *display,
>          return;
>      }
>      if (num_other_linked != display->common.base.clients_num) {
> -        RingItem *item, *next;
> +        GList *link, *next;
>          spice_debug("TODO: not O(n^2)");
> -        FOREACH_DCC(display, item, next, dcc) {
> +        FOREACH_DCC(display, link, next, dcc) {
>              int sent = 0;
>              DRAWABLE_FOREACH_DPI_SAFE(pos_after, dpi_link, dpi_next,
>              dpi_pos_after) {
>                  if (dpi_pos_after->dcc == dcc) {
> @@ -465,34 +465,32 @@ static int current_add_equal(DisplayChannel *display,
> DrawItem *item, TreeItem *
>  
>              DisplayChannelClient *dcc;
>              RedDrawablePipeItem *dpi;
> -            RingItem *worker_ring_item, *dpi_ring_item;
> +            RingItem *dpi_ring_item;
> +            GList *link;
>  
>              other_drawable->refs++;
>              current_remove_drawable(display, other_drawable);
>  
>              /* sending the drawable to clients that already received
>               * (or will receive) other_drawable */
> -            worker_ring_item =
> ring_get_head(&RED_CHANNEL(display)->clients);
> +            link = RED_CHANNEL(display)->clients;
>              dpi_ring_item = ring_get_head(&other_drawable->pipes);
>              /* dpi contains a sublist of dcc's, ordered the same */
> -            while (worker_ring_item) {
> -                dcc = SPICE_CONTAINEROF(worker_ring_item,
> DisplayChannelClient,
> -                                        common.base.channel_link);
> +            while (link) {
> +                dcc = link->data;
>                  dpi = SPICE_CONTAINEROF(dpi_ring_item, RedDrawablePipeItem,
>                  base);
> -                while (worker_ring_item && (!dpi || dcc != dpi->dcc)) {
> +                while (link && (!dpi || dcc != dpi->dcc)) {
>                      dcc_prepend_drawable(dcc, drawable);
> -                    worker_ring_item =
> ring_next(&RED_CHANNEL(display)->clients,
> -                                                 worker_ring_item);
> -                    dcc = SPICE_CONTAINEROF(worker_ring_item,
> DisplayChannelClient,
> -                                            common.base.channel_link);
> +                    link = link->next;
> +                    if (link)
> +                        dcc = link->data;
>                  }
>  
>                  if (dpi_ring_item) {
>                      dpi_ring_item = ring_next(&other_drawable->pipes,
>                      dpi_ring_item);
>                  }
> -                if (worker_ring_item) {
> -                    worker_ring_item =
> ring_next(&RED_CHANNEL(display)->clients,
> -                                                 worker_ring_item);
> +                if (link) {
> +                    link = link->next;
>                  }
>              }
>              /* not sending other_drawable where possible */
> @@ -1167,9 +1165,9 @@ int
> display_channel_wait_for_migrate_data(DisplayChannel *display)
>      }
>  
>      spice_debug(NULL);
> -    spice_warn_if_fail(channel->clients_num == 1);
> +    spice_warn_if_fail(g_list_length(channel->clients) == 1);
>  
> -    rcc = SPICE_CONTAINEROF(ring_get_head(&channel->clients),
> RedChannelClient, channel_link);
> +    rcc = g_list_nth_data(channel->clients, 0);
>  
>      red_channel_client_ref(rcc);
>      for (;;) {
> @@ -1206,24 +1204,24 @@ void
> display_channel_flush_all_surfaces(DisplayChannel *display)
>  
>  void display_channel_free_glz_drawables_to_free(DisplayChannel *display)
>  {
> -    RingItem *link, *next;
> +    GList *link, *next;
>      DisplayChannelClient *dcc;
>  
>      spice_return_if_fail(display);
>  
> -    DCC_FOREACH_SAFE(link, next, dcc, RED_CHANNEL(display)) {
> +    FOREACH_DCC(display, link, next, dcc) {
>          dcc_free_glz_drawables_to_free(dcc);
>      }
>  }
>  
>  void display_channel_free_glz_drawables(DisplayChannel *display)
>  {
> -    RingItem *link, *next;
> +    GList *link, *next;
>      DisplayChannelClient *dcc;
>  
>      spice_return_if_fail(display);
>  
> -    DCC_FOREACH_SAFE(link, next, dcc, RED_CHANNEL(display)) {
> +    FOREACH_DCC(display, link, next, dcc) {
>          dcc_free_glz_drawables(dcc);
>      }
>  }
> @@ -1266,11 +1264,11 @@ void display_channel_free_some(DisplayChannel
> *display)
>  {
>      int n = 0;
>      DisplayChannelClient *dcc;
> -    RingItem *item, *next;
> +    GList *link, *next;
>  
>      spice_debug("#draw=%d, #glz_draw=%d", display->drawable_count,
>                  display->glz_drawable_count);
> -    FOREACH_DCC(display, item, next, dcc) {
> +    FOREACH_DCC(display, link, next, dcc) {
>          GlzSharedDictionary *glz_dict = dcc ? dcc->glz_dict : NULL;
>  
>          if (glz_dict) {
> @@ -1285,7 +1283,7 @@ void display_channel_free_some(DisplayChannel *display)
>          free_one_drawable(display, TRUE);
>      }
>  
> -    FOREACH_DCC(display, item, next, dcc) {
> +    FOREACH_DCC(display, link, next, dcc) {
>          GlzSharedDictionary *glz_dict = dcc ? dcc->glz_dict : NULL;
>  
>          if (glz_dict) {
> @@ -1757,10 +1755,10 @@ void display_channel_update(DisplayChannel *display,
>  static void clear_surface_drawables_from_pipes(DisplayChannel *display, int
>  surface_id,
>                                                 int wait_if_used)
>  {
> -    RingItem *item, *next;
> +    GList *link, *next;
>      DisplayChannelClient *dcc;
>  
> -    FOREACH_DCC(display, item, next, dcc) {
> +    FOREACH_DCC(display, link, next, dcc) {
>          if (!dcc_clear_surface_drawables_from_pipe(dcc, surface_id,
>          wait_if_used)) {
>              red_channel_client_disconnect(RED_CHANNEL_CLIENT(dcc));
>          }
> @@ -1824,9 +1822,9 @@ void display_channel_destroy_surfaces(DisplayChannel
> *display)
>  static void send_create_surface(DisplayChannel *display, int surface_id, int
>  image_ready)
>  {
>      DisplayChannelClient *dcc;
> -    RingItem *item, *next;
> +    GList *link, *next;
>  
> -    FOREACH_DCC(display, item, next, dcc) {
> +    FOREACH_DCC(display, link, next, dcc) {
>          dcc_create_surface(dcc, surface_id);
>          if (image_ready)
>              dcc_push_surface_image(dcc, surface_id);
> diff --git a/server/display-channel.h b/server/display-channel.h
> index 647d8c0..cec8c8e 100644
> --- a/server/display-channel.h
> +++ b/server/display-channel.h
> @@ -226,14 +226,14 @@ struct DisplayChannel {
>      stat_info_t lz4_stat;
>  };
>  
> -#define LINK_TO_DCC(ptr) SPICE_CONTAINEROF(ptr, DisplayChannelClient,   \
> -                                           common.base.channel_link)
> -#define DCC_FOREACH_SAFE(link, next, dcc, channel)                      \
> -    SAFE_FOREACH(link, next, channel,  &(channel)->clients, dcc,
> LINK_TO_DCC(link))
> -
> -
> -#define FOREACH_DCC(display_channel, link, next, dcc)                   \
> -    DCC_FOREACH_SAFE(link, next, dcc, RED_CHANNEL(display_channel))
> +#define FOREACH_DCC(channel, _link, _next, _data)                   \
> +    for (_link = (channel ? RED_CHANNEL(channel)->clients : NULL), \
> +         _next = (_link ? _link->next : NULL), \
> +         _data = (_link ? _link->data : NULL); \
> +         _link; \
> +         _link = _next, \
> +         _next = (_link ? _link->next : NULL), \
> +         _data = (_link ? _link->data : NULL))
>  
>  static inline int get_stream_id(DisplayChannel *display, Stream *stream)
>  {
> diff --git a/server/main-channel.c b/server/main-channel.c
> index 9b1bd22..5b5588c 100644
> --- a/server/main-channel.c
> +++ b/server/main-channel.c
> @@ -42,12 +42,12 @@ static void
> main_channel_client_on_disconnect(RedChannelClient *rcc)
>  
>  RedClient *main_channel_get_client_by_link_id(MainChannel *main_chan,
>  uint32_t connection_id)
>  {
> -    RingItem *link;
> +    GList *link;
>      MainChannelClient *mcc;
>      RedChannelClient *rcc;
>  
> -    RING_FOREACH(link, &main_chan->base.clients) {
> -        rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link);
> +    for (link = main_chan->base.clients; link != NULL; link = link->next) {
> +        rcc = link->data;
>          mcc = (MainChannelClient*) rcc;
>          if (main_channel_client_get_connection_id(mcc) == connection_id) {
>              return rcc->client;
> @@ -333,11 +333,10 @@ MainChannel* main_channel_new(RedsState *reds)
>  
>  static int main_channel_connect_semi_seamless(MainChannel *main_channel)
>  {
> -    RingItem *client_link;
> +    GList *link;
>  
> -    RING_FOREACH(client_link, &main_channel->base.clients) {
> -        RedChannelClient *rcc = SPICE_CONTAINEROF(client_link,
> RedChannelClient,
> -                                                    channel_link);
> +    for (link = main_channel->base.clients; link != NULL; link = link->next)
> {
> +        RedChannelClient *rcc = link->data;
>          MainChannelClient *mcc = (MainChannelClient*)rcc;
>          if (main_channel_client_connect_semi_seamless(mcc))
>              main_channel->num_clients_mig_wait++;
> @@ -347,13 +346,12 @@ static int
> main_channel_connect_semi_seamless(MainChannel *main_channel)
>  
>  static int main_channel_connect_seamless(MainChannel *main_channel)
>  {
> -    RingItem *client_link;
> +    GList *link;
>  
> -    spice_assert(main_channel->base.clients_num == 1);
> +    spice_assert(g_list_length(main_channel->base.clients) == 1);
>  
> -    RING_FOREACH(client_link, &main_channel->base.clients) {
> -        RedChannelClient *rcc = SPICE_CONTAINEROF(client_link,
> RedChannelClient,
> -                                                    channel_link);
> +    for (link = main_channel->base.clients; link != NULL; link = link->next)
> {
> +        RedChannelClient *rcc = link->data;
>          MainChannelClient *mcc = (MainChannelClient*)rcc;
>          main_channel_client_connect_seamless(mcc);
>          main_channel->num_clients_mig_wait++;
> @@ -374,11 +372,9 @@ int main_channel_migrate_connect(MainChannel
> *main_channel, RedsMigSpice *mig_ta
>      if (!try_seamless) {
>          return main_channel_connect_semi_seamless(main_channel);
>      } else {
> -        RingItem *client_item;
>          RedChannelClient *rcc;
>  
> -        client_item = ring_get_head(&main_channel->base.clients);
> -        rcc = SPICE_CONTAINEROF(client_item, RedChannelClient,
> channel_link);
> +        rcc = g_list_nth_data(main_channel->base.clients, 0);
>  
>          if (!red_channel_client_test_remote_cap(rcc,
>                                                  SPICE_MAIN_CAP_SEAMLESS_MIGRATE))
>                                                  {
> @@ -392,11 +388,10 @@ int main_channel_migrate_connect(MainChannel
> *main_channel, RedsMigSpice *mig_ta
>  
>  void main_channel_migrate_cancel_wait(MainChannel *main_chan)
>  {
> -    RingItem *client_link;
> +    GList *link;
>  
> -    RING_FOREACH(client_link, &main_chan->base.clients) {
> -        RedChannelClient *rcc = SPICE_CONTAINEROF(client_link,
> RedChannelClient,
> -                                                    channel_link);
> +    for (link = main_chan->base.clients; link != NULL; link = link->next) {
> +        RedChannelClient *rcc = link->data;
>          MainChannelClient *mcc = (MainChannelClient*)rcc;
>          main_channel_client_migrate_cancel_wait(mcc);
>      }
> @@ -405,19 +400,18 @@ void main_channel_migrate_cancel_wait(MainChannel
> *main_chan)
>  
>  int main_channel_migrate_src_complete(MainChannel *main_chan, int success)
>  {
> -    RingItem *client_link;
> +    GList *link;
>      int semi_seamless_count = 0;
>  
>      spice_printerr("");
>  
> -    if (ring_is_empty(&main_chan->base.clients)) {
> +    if (!main_chan->base.clients) {
>          spice_printerr("no peer connected");
>          return 0;
>      }
>  
> -    RING_FOREACH(client_link, &main_chan->base.clients) {
> -        RedChannelClient *rcc = SPICE_CONTAINEROF(client_link,
> RedChannelClient,
> -                                                    channel_link);
> +    for (link = main_chan->base.clients; link != NULL; link = link->next) {
> +        RedChannelClient *rcc = link->data;
>          MainChannelClient *mcc = (MainChannelClient*)rcc;
>          if (main_channel_client_migrate_src_complete(mcc, success))
>              semi_seamless_count++;
> diff --git a/server/red-channel.c b/server/red-channel.c
> index 2ec65e8..07b0a7b 100644
> --- a/server/red-channel.c
> +++ b/server/red-channel.c
> @@ -327,14 +327,7 @@ void red_channel_client_receive(RedChannelClient *rcc)
>  
>  void red_channel_receive(RedChannel *channel)
>  {
> -    RingItem *link;
> -    RingItem *next;
> -    RedChannelClient *rcc;
> -
> -    RING_FOREACH_SAFE(link, next, &channel->clients) {
> -        rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link);
> -        red_channel_client_receive(rcc);
> -    }
> +    g_list_foreach(channel->clients, (GFunc)red_channel_client_receive,
> NULL);
>  }
>  
>  static void red_peer_handle_outgoing(RedsStream *stream, OutgoingHandler
>  *handler)
> @@ -637,8 +630,7 @@ static void
> red_channel_client_pipe_remove(RedChannelClient *rcc, RedPipeItem *i
>  static void red_channel_add_client(RedChannel *channel, RedChannelClient
>  *rcc)
>  {
>      spice_assert(rcc);
> -    ring_add(&channel->clients, &rcc->channel_link);
> -    channel->clients_num++;
> +    channel->clients = g_list_append(channel->clients, rcc);

ring_add does a prepend, not an append

>  }
>  
>  static void red_channel_client_set_remote_caps(RedChannelClient* rcc,
> @@ -676,10 +668,10 @@ int red_channel_client_test_remote_cap(RedChannelClient
> *rcc, uint32_t cap)
>  
>  int red_channel_test_remote_common_cap(RedChannel *channel, uint32_t cap)
>  {
> -    RingItem *link;
> +    GList *link;
>  
> -    RING_FOREACH(link, &channel->clients) {
> -        RedChannelClient *rcc = SPICE_CONTAINEROF(link, RedChannelClient,
> channel_link);
> +    for (link = channel->clients; link != NULL; link = link->next) {
> +        RedChannelClient *rcc = link->data;
>  
>          if (!red_channel_client_test_remote_common_cap(rcc, cap)) {
>              return FALSE;
> @@ -690,10 +682,10 @@ int red_channel_test_remote_common_cap(RedChannel
> *channel, uint32_t cap)
>  
>  int red_channel_test_remote_cap(RedChannel *channel, uint32_t cap)
>  {
> -    RingItem *link;
> +    GList *link;
>  
> -    RING_FOREACH(link, &channel->clients) {
> -        RedChannelClient *rcc = SPICE_CONTAINEROF(link, RedChannelClient,
> channel_link);
> +    for (link = channel->clients; link != NULL; link = link->next) {
> +        RedChannelClient *rcc = link->data;
>  
>          if (!red_channel_client_test_remote_cap(rcc, cap)) {
>              return FALSE;
> @@ -901,7 +893,7 @@ RedChannelClient *red_channel_client_create(int size,
> RedChannel *channel, RedCl
>                                             stream->socket,
>                                             SPICE_WATCH_EVENT_READ,
>                                             red_channel_client_event, rcc);
> -    rcc->id = channel->clients_num;
> +    rcc->id = g_list_length(channel->clients);
>      red_channel_add_client(channel, rcc);
>      red_client_add_channel(client, rcc);
>      red_channel_ref(channel);
> @@ -970,16 +962,17 @@ int
> red_channel_client_is_waiting_for_migrate_data(RedChannelClient *rcc)
>  int red_channel_is_waiting_for_migrate_data(RedChannel *channel)
>  {
>      RedChannelClient *rcc;
> +    guint n_clients = g_list_length(channel->clients);
>  
>      if (!red_channel_is_connected(channel)) {
>          return FALSE;
>      }
>  
> -    if (channel->clients_num > 1) {
> +    if (n_clients > 1) {
>          return FALSE;
>      }
> -    spice_assert(channel->clients_num == 1);
> -    rcc = SPICE_CONTAINEROF(ring_get_head(&channel->clients),
> RedChannelClient, channel_link);
> +    spice_assert(n_clients == 1);
> +    rcc = g_list_nth_data(channel->clients, 0);
>      return red_channel_client_is_waiting_for_migrate_data(rcc);
>  }
>  
> @@ -1038,7 +1031,6 @@ RedChannel *red_channel_create(int size,
>  
>      channel->reds = reds;
>      channel->core = core;
> -    ring_init(&channel->clients);
>  
>      // TODO: send incoming_cb as parameters instead of duplicating?
>      channel->incoming_cb.alloc_msg_buf =
>      (alloc_msg_recv_buf_proc)channel_cbs->alloc_recv_buf;
> @@ -1105,7 +1097,6 @@ RedChannel *red_channel_create_dummy(int size,
> RedsState *reds, uint32_t type, u
>      channel->refs = 1;
>      channel->reds = reds;
>      channel->core = &dummy_core;
> -    ring_init(&channel->clients);
>      client_cbs.connect = red_channel_client_default_connect;
>      client_cbs.disconnect = red_channel_client_default_disconnect;
>      client_cbs.migrate = red_channel_client_default_migrate;
> @@ -1272,17 +1263,11 @@ void red_channel_client_destroy(RedChannelClient
> *rcc)
>  
>  void red_channel_destroy(RedChannel *channel)
>  {
> -    RingItem *link;
> -    RingItem *next;
> -
>      if (!channel) {
>          return;
>      }
> -    RING_FOREACH_SAFE(link, next, &channel->clients) {
> -        red_channel_client_destroy(
> -            SPICE_CONTAINEROF(link, RedChannelClient, channel_link));
> -    }
>  
> +    g_list_foreach(channel->clients, (GFunc)red_channel_client_destroy,
> NULL);
>      red_channel_unref(channel);
>  }
>  
> @@ -1305,12 +1290,7 @@ void red_channel_client_send(RedChannelClient *rcc)
>  
>  void red_channel_send(RedChannel *channel)
>  {
> -    RingItem *link;
> -    RingItem *next;
> -
> -    RING_FOREACH_SAFE(link, next, &channel->clients) {
> -        red_channel_client_send(SPICE_CONTAINEROF(link, RedChannelClient,
> channel_link));
> -    }
> +    g_list_foreach(channel->clients, (GFunc)red_channel_client_send, NULL);
>  }
>  
>  static inline int red_channel_client_waiting_for_ack(RedChannelClient *rcc)
> @@ -1365,17 +1345,11 @@ void red_channel_client_push(RedChannelClient *rcc)
>  
>  void red_channel_push(RedChannel *channel)
>  {
> -    RingItem *link;
> -    RingItem *next;
> -    RedChannelClient *rcc;
> -
>      if (!channel) {
>          return;
>      }
> -    RING_FOREACH_SAFE(link, next, &channel->clients) {
> -        rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link);
> -        red_channel_client_push(rcc);
> -    }
> +
> +    g_list_foreach(channel->clients, (GFunc)red_channel_client_push, NULL);
>  }
>  
>  int red_channel_client_get_roundtrip_ms(RedChannelClient *rcc)
> @@ -1396,13 +1370,7 @@ static void
> red_channel_client_init_outgoing_messages_window(RedChannelClient *r
>  // specific
>  void red_channel_init_outgoing_messages_window(RedChannel *channel)
>  {
> -    RingItem *link;
> -    RingItem *next;
> -
> -    RING_FOREACH_SAFE(link, next, &channel->clients) {
> -        red_channel_client_init_outgoing_messages_window(
> -            SPICE_CONTAINEROF(link, RedChannelClient, channel_link));
> -    }
> +    g_list_foreach(channel->clients,
> (GFunc)red_channel_client_init_outgoing_messages_window, NULL);
>  }
>  
>  static void red_channel_handle_migrate_flush_mark(RedChannelClient *rcc)
> @@ -1719,15 +1687,16 @@ void
> red_channel_client_pipe_add_type(RedChannelClient *rcc, int pipe_item_type)
>      red_channel_client_push(rcc);
>  }
>  
> -void red_channel_pipes_add_type(RedChannel *channel, int pipe_item_type)
> +static void red_channel_client_pipe_add_type_proxy(gpointer data, gpointer
> user_data)
>  {
> -    RingItem *link, *next;
> +    int type = GPOINTER_TO_INT(user_data);
> +    red_channel_client_pipe_add_type(data, type);
> +}
>  
> -    RING_FOREACH_SAFE(link, next, &channel->clients) {
> -        red_channel_client_pipe_add_type(
> -            SPICE_CONTAINEROF(link, RedChannelClient, channel_link),
> -            pipe_item_type);
> -    }
> +void red_channel_pipes_add_type(RedChannel *channel, int pipe_item_type)
> +{
> +    g_list_foreach(channel->clients, red_channel_client_pipe_add_type_proxy,
> +                   GINT_TO_POINTER(pipe_item_type));
>  }
>  
>  void red_channel_client_pipe_add_empty_msg(RedChannelClient *rcc, int
>  msg_type)
> @@ -1740,21 +1709,22 @@ void
> red_channel_client_pipe_add_empty_msg(RedChannelClient *rcc, int msg_type)
>      red_channel_client_push(rcc);
>  }
>  
> -void red_channel_pipes_add_empty_msg(RedChannel *channel, int msg_type)
> +static void red_channel_client_pipe_add_empty_msg_proxy(gpointer data,
> gpointer user_data)
>  {
> -    RingItem *link, *next;
> +    int type = GPOINTER_TO_INT(user_data);
> +    red_channel_client_pipe_add_empty_msg(data, type);
> +}
>  
> -    RING_FOREACH_SAFE(link, next, &channel->clients) {
> -        red_channel_client_pipe_add_empty_msg(
> -            SPICE_CONTAINEROF(link, RedChannelClient, channel_link),
> -            msg_type);
> -    }
> +void red_channel_pipes_add_empty_msg(RedChannel *channel, int msg_type)
> +{
> +    g_list_foreach(channel->clients,
> red_channel_client_pipe_add_empty_msg_proxy, GINT_TO_POINTER(msg_type));
>  }
>  
>  int red_channel_client_is_connected(RedChannelClient *rcc)
>  {
>      if (!rcc->dummy) {
> -        return ring_item_is_linked(&rcc->channel_link);
> +        return rcc->channel
> +            && (g_list_find(rcc->channel->clients, rcc) != NULL);
>      } else {
>          return rcc->dummy_connected;
>      }
> @@ -1762,7 +1732,7 @@ int red_channel_client_is_connected(RedChannelClient
> *rcc)
>  
>  int red_channel_is_connected(RedChannel *channel)
>  {
> -    return channel && (channel->clients_num > 0);
> +    return channel && channel->clients;
>  }
>  
>  static void red_channel_client_clear_sent_item(RedChannelClient *rcc)
> @@ -1798,6 +1768,8 @@ void
> red_channel_client_ack_set_client_window(RedChannelClient *rcc, int client_
>  
>  static void red_channel_remove_client(RedChannelClient *rcc)
>  {
> +    GList *link;
> +
>      if (!pthread_equal(pthread_self(), rcc->channel->thread_id)) {
>          spice_warning("channel type %d id %d - "
>                        "channel->thread_id (0x%lx) != pthread_self (0x%lx)."
> @@ -1806,11 +1778,11 @@ static void
> red_channel_remove_client(RedChannelClient *rcc)
>                        rcc->channel->type, rcc->channel->id,
>                        rcc->channel->thread_id, pthread_self());
>      }
> -    spice_return_if_fail(ring_item_is_linked(&rcc->channel_link));
> +    spice_return_if_fail(rcc->channel);
> +    link = g_list_find(rcc->channel->clients, rcc);
> +    spice_return_if_fail(link != NULL);
>  
> -    ring_remove(&rcc->channel_link);
> -    spice_assert(rcc->channel->clients_num > 0);
> -    rcc->channel->clients_num--;
> +    rcc->channel->clients = g_list_remove_link(rcc->channel->clients, link);
>      // TODO: should we set rcc->channel to NULL???
>  }
>  
> @@ -1824,11 +1796,12 @@ static void
> red_client_remove_channel(RedChannelClient *rcc)
>  
>  static void red_channel_client_disconnect_dummy(RedChannelClient *rcc)
>  {
> +    GList *link;
>      spice_assert(rcc->dummy);
> -    if (ring_item_is_linked(&rcc->channel_link)) {
> +    if (rcc->channel && (link = g_list_find(rcc->channel->clients, rcc))) {
>          spice_printerr("rcc=%p (channel=%p type=%d id=%d)", rcc,
>          rcc->channel,
>                         rcc->channel->type, rcc->channel->id);
> -        red_channel_remove_client(rcc);
> +        red_channel_remove_client(link->data);
>      }
>      rcc->dummy_connected = FALSE;
>  }
> @@ -1863,13 +1836,7 @@ void red_channel_client_disconnect(RedChannelClient
> *rcc)
>  
>  void red_channel_disconnect(RedChannel *channel)
>  {
> -    RingItem *link;
> -    RingItem *next;
> -
> -    RING_FOREACH_SAFE(link, next, &channel->clients) {
> -        red_channel_client_disconnect(
> -            SPICE_CONTAINEROF(link, RedChannelClient, channel_link));
> -    }
> +    g_list_foreach(channel->clients, (GFunc)red_channel_client_disconnect,
> NULL);
>  }
>  
>  RedChannelClient *red_channel_client_create_dummy(int size,
> @@ -1919,26 +1886,24 @@ error:
>  
>  void red_channel_apply_clients(RedChannel *channel, channel_client_callback
>  cb)
>  {
> -    RingItem *link;
> -    RingItem *next;
> -    RedChannelClient *rcc;
> +    g_list_foreach(channel->clients, (GFunc)cb, NULL);
> +}
>  
> -    RING_FOREACH_SAFE(link, next, &channel->clients) {
> -        rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link);
> -        cb(rcc);
> -    }
> +void red_channel_apply_clients_data(RedChannel *channel,
> channel_client_callback_data cb, void *data)
> +{
> +    g_list_foreach(channel->clients, (GFunc)cb, data);
>  }
>  
>  int red_channel_all_blocked(RedChannel *channel)
>  {
> -    RingItem *link;
> +    GList *link;
>      RedChannelClient *rcc;
>  
> -    if (!channel || channel->clients_num == 0) {
> +    if (!channel || !channel->clients) {
>          return FALSE;
>      }
> -    RING_FOREACH(link, &channel->clients) {
> -        rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link);
> +    for (link = channel->clients; link != NULL; link = link->next) {
> +        rcc = link->data;
>          if (!rcc->send_data.blocked) {
>              return FALSE;
>          }
> @@ -1948,11 +1913,11 @@ int red_channel_all_blocked(RedChannel *channel)
>  
>  int red_channel_any_blocked(RedChannel *channel)
>  {
> -    RingItem *link;
> +    GList *link;
>      RedChannelClient *rcc;
>  
> -    RING_FOREACH(link, &channel->clients) {
> -        rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link);
> +    for (link = channel->clients; link != NULL; link = link->next) {
> +        rcc = link->data;
>          if (rcc->send_data.blocked) {
>              return TRUE;
>          }
> @@ -1995,20 +1960,23 @@ void
> red_channel_client_set_header_sub_list(RedChannelClient *rcc, uint32_t sub_
>  
>  int red_channel_get_first_socket(RedChannel *channel)
>  {
> -    if (!channel || channel->clients_num == 0) {
> +    RedChannelClient *rcc;
> +
> +    if (!channel || !channel->clients) {
>          return -1;
>      }
> -    return SPICE_CONTAINEROF(ring_get_head(&channel->clients),
> -                             RedChannelClient,
> channel_link)->stream->socket;
> +    rcc = g_list_nth_data(channel->clients, 0);
> +
> +    return rcc->stream->socket;
>  }
>  
>  int red_channel_no_item_being_sent(RedChannel *channel)
>  {
> -    RingItem *link;
> +    GList *link;
>      RedChannelClient *rcc;
>  
> -    RING_FOREACH(link, &channel->clients) {
> -        rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link);
> +    for (link = channel->clients; link != NULL; link = link->next) {
> +        rcc = link->data;
>          if (!red_channel_client_no_item_being_sent(rcc)) {
>              return FALSE;
>          }
> @@ -2245,7 +2213,7 @@ static int red_channel_pipes_create_batch(RedChannel
> *channel,
>                                  new_pipe_item_t creator, void *data,
>                                  rcc_item_t pipe_add)
>  {
> -    RingItem *link, *next;
> +    GList *link, *next;
>      RedChannelClient *rcc;
>      RedPipeItem *item;
>      int num = 0, n = 0;
> @@ -2253,13 +2221,16 @@ static int red_channel_pipes_create_batch(RedChannel
> *channel,
>      spice_assert(creator != NULL);
>      spice_assert(pipe_add != NULL);
>  
> -    RING_FOREACH_SAFE(link, next, &channel->clients) {
> -        rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link);
> +    link = channel->clients;
> +    while (link != NULL) {
> +        next = link->next;
> +        rcc = link->data;
>          item = (*creator)(rcc, data, num++);
>          if (item) {
>              (*pipe_add)(rcc, item);
>              n++;
>          }
> +        link = next;
>      }
>  
>      return n;
> @@ -2289,12 +2260,12 @@ void red_channel_pipes_new_add_tail(RedChannel
> *channel, new_pipe_item_t creator
>  
>  uint32_t red_channel_max_pipe_size(RedChannel *channel)
>  {
> -    RingItem *link;
> +    GList *link;
>      RedChannelClient *rcc;
>      uint32_t pipe_size = 0;
>  
> -    RING_FOREACH(link, &channel->clients) {
> -        rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link);
> +    for (link = channel->clients; link != NULL; link = link->next) {
> +        rcc = link->data;
>          pipe_size = MAX(pipe_size, rcc->pipe_size);
>      }
>      return pipe_size;
> @@ -2302,12 +2273,12 @@ uint32_t red_channel_max_pipe_size(RedChannel
> *channel)
>  
>  uint32_t red_channel_min_pipe_size(RedChannel *channel)
>  {
> -    RingItem *link;
> +    GList *link;
>      RedChannelClient *rcc;
>      uint32_t pipe_size = ~0;
>  
> -    RING_FOREACH(link, &channel->clients) {
> -        rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link);
> +    for (link = channel->clients; link != NULL; link = link->next) {
> +        rcc = link->data;
>          pipe_size = MIN(pipe_size, rcc->pipe_size);
>      }
>      return pipe_size == ~0 ? 0 : pipe_size;
> @@ -2315,12 +2286,12 @@ uint32_t red_channel_min_pipe_size(RedChannel
> *channel)
>  
>  uint32_t red_channel_sum_pipes_size(RedChannel *channel)
>  {
> -    RingItem *link;
> +    GList *link;
>      RedChannelClient *rcc;
>      uint32_t sum = 0;
>  
> -    RING_FOREACH(link, &channel->clients) {
> -        rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link);
> +    for (link = channel->clients; link != NULL; link = link->next) {
> +        rcc = link->data;
>          sum += rcc->pipe_size;
>      }
>      return sum;
> diff --git a/server/red-channel.h b/server/red-channel.h
> index 63cb2d9..c3ed111 100644
> --- a/server/red-channel.h
> +++ b/server/red-channel.h
> @@ -308,7 +308,7 @@ struct RedChannel {
>      // However RCC still holds a reference to the Channel.
>      // Maybe replace these logic with ref count?
>      // TODO: rename to 'connected_clients'?
> -    Ring clients;
> +    GList *clients;
>      uint32_t clients_num;
>  
>      OutgoingHandlerInterface outgoing_cb;
> @@ -549,6 +549,7 @@ uint32_t red_channel_sum_pipes_size(RedChannel *channel);
>  typedef void (*channel_client_callback)(RedChannelClient *rcc);
>  typedef void (*channel_client_callback_data)(RedChannelClient *rcc, void
>  *data);
>  void red_channel_apply_clients(RedChannel *channel, channel_client_callback
>  v);
> +void red_channel_apply_clients_data(RedChannel *channel,
> channel_client_callback_data v, void * data);
>  struct RedsState* red_channel_get_server(RedChannel *channel);
>  
>  struct RedClient {
> diff --git a/server/red-worker.c b/server/red-worker.c
> index 8d8073e..86f1645 100644
> --- a/server/red-worker.c
> +++ b/server/red-worker.c
> @@ -500,7 +500,7 @@ static void guest_set_client_capabilities(RedWorker
> *worker)
>      int i;
>      DisplayChannelClient *dcc;
>      RedChannelClient *rcc;
> -    RingItem *link, *next;
> +    GList *link, *next;
>      uint8_t caps[SPICE_CAPABILITIES_SIZE] = { 0 };
>      int caps_available[] = {
>          SPICE_DISPLAY_CAP_SIZED_STREAM,
> @@ -533,7 +533,7 @@ static void guest_set_client_capabilities(RedWorker
> *worker)
>          for (i = 0 ; i < sizeof(caps_available) / sizeof(caps_available[0]);
>          ++i) {
>              SET_CAP(caps, caps_available[i]);
>          }
> -        DCC_FOREACH_SAFE(link, next, dcc,
> RED_CHANNEL(worker->display_channel)) {
> +        FOREACH_DCC(worker->display_channel, link, next, dcc) {
>              rcc = (RedChannelClient *)dcc;
>              for (i = 0 ; i < sizeof(caps_available) /
>              sizeof(caps_available[0]); ++i) {
>                  if (!red_channel_client_test_remote_cap(rcc,
>                  caps_available[i]))
> @@ -646,7 +646,7 @@ static void display_update_monitors_config(DisplayChannel
> *display,
>  static void red_worker_push_monitors_config(RedWorker *worker)
>  {
>      DisplayChannelClient *dcc;
> -    RingItem *item, *next;
> +    GList *item, *next;
>  
>      FOREACH_DCC(worker->display_channel, item, next, dcc) {
>          dcc_push_monitors_config(dcc);
> diff --git a/server/red-worker.h b/server/red-worker.h
> index d7525e0..63be8b5 100644
> --- a/server/red-worker.h
> +++ b/server/red-worker.h
> @@ -75,19 +75,16 @@ static inline void red_pipe_add_verb(RedChannelClient*
> rcc, uint16_t verb)
>      red_channel_client_pipe_add(rcc, &item->base);
>  }
>  
> -#define LINK_TO_RCC(ptr) SPICE_CONTAINEROF(ptr, RedChannelClient,
> channel_link)
> -#define RCC_FOREACH_SAFE(link, next, rcc, channel) \
> -    SAFE_FOREACH(link, next, channel,  &(channel)->clients, rcc,
> LINK_TO_RCC(link))
> +static inline void red_pipe_add_verb_proxy(RedChannelClient *rcc, gpointer
> data)
> +{
> +    uint16_t verb = GPOINTER_TO_UINT(data);
> +    red_pipe_add_verb(rcc, verb);
> +}
>  
>  
>  static inline void red_pipes_add_verb(RedChannel *channel, uint16_t verb)
>  {
> -    RedChannelClient *rcc;
> -    RingItem *link, *next;
> -
> -    RCC_FOREACH_SAFE(link, next, rcc, channel) {
> -        red_pipe_add_verb(rcc, verb);
> -    }
> +    red_channel_apply_clients_data(channel, red_pipe_add_verb_proxy,
> GUINT_TO_POINTER(verb));
>  }
>  
>  RedWorker* red_worker_new(QXLInstance *qxl,
> diff --git a/server/stream.c b/server/stream.c
> index 4c733de..2ef3813 100644
> --- a/server/stream.c
> +++ b/server/stream.c
> @@ -96,13 +96,13 @@ static RedPipeItem *stream_destroy_item_new(StreamAgent
> *agent)
>  void stream_stop(DisplayChannel *display, Stream *stream)
>  {
>      DisplayChannelClient *dcc;
> -    RingItem *item, *next;
> +    GList *link, *next;
>  
>      spice_return_if_fail(ring_item_is_linked(&stream->link));
>      spice_return_if_fail(!stream->current);
>  
>      spice_debug("stream %d", get_stream_id(display, stream));
> -    FOREACH_DCC(display, item, next, dcc) {
> +    FOREACH_DCC(display, link, next, dcc) {
>          StreamAgent *stream_agent;
>  
>          stream_agent = &dcc->stream_agents[get_stream_id(display, stream)];
> @@ -288,7 +288,7 @@ static int is_next_stream_frame(DisplayChannel *display,
>  static void attach_stream(DisplayChannel *display, Drawable *drawable,
>  Stream *stream)
>  {
>      DisplayChannelClient *dcc;
> -    RingItem *item, *next;
> +    GList *link, *next;
>  
>      spice_assert(drawable && stream);
>      spice_assert(!drawable->stream && !stream->current);
> @@ -307,7 +307,7 @@ static void attach_stream(DisplayChannel *display,
> Drawable *drawable, Stream *s
>          stream->num_input_frames++;
>      }
>  
> -    FOREACH_DCC(display, item, next, dcc) {
> +    FOREACH_DCC(display, link, next, dcc) {
>          StreamAgent *agent;
>          QRegion clip_in_draw_dest;
>  
> @@ -349,6 +349,7 @@ static void before_reattach_stream(DisplayChannel
> *display,
>      int index;
>      StreamAgent *agent;
>      RingItem *ring_item, *next;
> +    GList *link, *link_next;
>  
>      spice_return_if_fail(stream->current);
>  
> @@ -384,7 +385,7 @@ static void before_reattach_stream(DisplayChannel
> *display,
>      }
>  
>  
> -    FOREACH_DCC(display, ring_item, next, dcc) {
> +    FOREACH_DCC(display, link, link_next, dcc) {
>          double drop_factor;
>  
>          agent = &dcc->stream_agents[index];
> @@ -429,7 +430,7 @@ static Stream
> *display_channel_stream_try_new(DisplayChannel *display)
>  static void display_channel_create_stream(DisplayChannel *display, Drawable
>  *drawable)
>  {
>      DisplayChannelClient *dcc;
> -    RingItem *dcc_ring_item, *next;
> +    GList *link, *next;
>      Stream *stream;
>      SpiceRect* src_rect;
>  
> @@ -466,7 +467,7 @@ static void display_channel_create_stream(DisplayChannel
> *display, Drawable *dra
>      stream->input_fps_start_time = drawable->creation_time;
>      display->streams_size_total += stream->width * stream->height;
>      display->stream_count++;
> -    FOREACH_DCC(display, dcc_ring_item, next, dcc) {
> +    FOREACH_DCC(display, link, next, dcc) {
>          dcc_create_stream(dcc, stream);
>      }
>      spice_debug("stream %d %dx%d (%d, %d) (%d, %d) %u fps",
> @@ -867,10 +868,10 @@ clear_vis_region:
>  static void detach_stream_gracefully(DisplayChannel *display, Stream
>  *stream,
>                                       Drawable *update_area_limit)
>  {
> -    RingItem *item, *next;
> +    GList *link, *next;
>      DisplayChannelClient *dcc;
>  
> -    FOREACH_DCC(display, item, next, dcc) {
> +    FOREACH_DCC(display, link, next, dcc) {
>          dcc_detach_stream_gracefully(dcc, stream, update_area_limit);
>      }
>      if (stream->current) {
> @@ -892,7 +893,7 @@ void stream_detach_behind(DisplayChannel *display,
> QRegion *region, Drawable *dr
>  {
>      Ring *ring = &display->streams;
>      RingItem *item = ring_get_head(ring);
> -    RingItem *dcc_ring_item, *next;
> +    GList *link, *next;
>      DisplayChannelClient *dcc;
>      bool is_connected = red_channel_is_connected(RED_CHANNEL(display));
>  
> @@ -901,7 +902,7 @@ void stream_detach_behind(DisplayChannel *display,
> QRegion *region, Drawable *dr
>          int detach = 0;
>          item = ring_next(ring, item);
>  
> -        FOREACH_DCC(display, dcc_ring_item, next, dcc) {
> +        FOREACH_DCC(display, link, next, dcc) {
>              StreamAgent *agent = &dcc->stream_agents[get_stream_id(display,
>              stream)];
>  
>              if (region_intersects(&agent->vis_region, region)) {

Beside the append change,

Acked-by: Frediano Ziglio <fziglio at redhat.com>

Frediano


More information about the Spice-devel mailing list