[Spice-devel] [PATCH 06/14] Replace RedClient::channels with GList

[Christophe Fergeau]

Hey,

On Tue, May 03, 2016 at 03:00:22PM -0500, Jonathon Jongsma wrote:
> @@ -2086,30 +2083,33 @@ void red_client_set_migration_seamless(RedClient *client) // dest
>  
>  void red_client_migrate(RedClient *client)
>  {
> -    RingItem *link, *next;
> +    GList *link, *next;
>      RedChannelClient *rcc;
>  
> -    spice_printerr("migrate client with #channels %d", client->channels_num);
> +    spice_printerr("migrate client with #channels %d", g_list_length(client->channels));
>      if (!pthread_equal(pthread_self(), client->thread_id)) {
>          spice_warning("client->thread_id (0x%lx) != pthread_self (0x%lx)."
>                        "If one of the threads is != io-thread && != vcpu-thread,"
>                        " this might be a BUG",
>                        client->thread_id, pthread_self());
>      }
> -    RING_FOREACH_SAFE(link, next, &client->channels) {
> -        rcc = SPICE_CONTAINEROF(link, RedChannelClient, client_link);
> +    link = client->channels;
> +    while (link) {

When replacing RING_FOREACH_SAFE, you could declare 'next' in the loop
block rather than at the same time as 'link'.

Christophe

> +        next = link->next;
> +        rcc = link->data;
>          if (red_channel_client_is_connected(rcc)) {
>              rcc->channel->client_cbs.migrate(rcc);
>          }
> +        link = next;
>      }
>  }
>  
>  void red_client_destroy(RedClient *client)
>  {
> -    RingItem *link, *next;
> +    GList *link, *next;
>      RedChannelClient *rcc;
>  
> -    spice_printerr("destroy client %p with #channels=%d", client, client->channels_num);
> +    spice_printerr("destroy client %p with #channels=%d", client, g_list_length(client->channels));
>      if (!pthread_equal(pthread_self(), client->thread_id)) {
>          spice_warning("client->thread_id (0x%lx) != pthread_self (0x%lx)."
>                        "If one of the threads is != io-thread && != vcpu-thread,"
> @@ -2117,10 +2117,12 @@ void red_client_destroy(RedClient *client)
>                        client->thread_id,
>                        pthread_self());
>      }
> -    RING_FOREACH_SAFE(link, next, &client->channels) {
> +    link = client->channels;
> +    while (link) {
> +        next = link->next;
>          // some channels may be in other threads, so disconnection
>          // is not synchronous.
> -        rcc = SPICE_CONTAINEROF(link, RedChannelClient, client_link);
> +        rcc = link->data;
>          rcc->destroying = 1;
>          // some channels may be in other threads. However we currently
>          // assume disconnect is synchronous (we changed the dispatcher
> @@ -2132,6 +2134,7 @@ void red_client_destroy(RedClient *client)
>          spice_assert(rcc->pipe_size == 0);
>          spice_assert(rcc->send_data.size == 0);
>          red_channel_client_destroy(rcc);
> +        link = next;
>      }
>      red_client_unref(client);
>  }
> @@ -2139,12 +2142,12 @@ void red_client_destroy(RedClient *client)
>  /* client->lock should be locked */
>  static RedChannelClient *red_client_get_channel(RedClient *client, int type, int id)
>  {
> -    RingItem *link;
> +    GList *link;
>      RedChannelClient *rcc;
>      RedChannelClient *ret = NULL;
>  
> -    RING_FOREACH(link, &client->channels) {
> -        rcc = SPICE_CONTAINEROF(link, RedChannelClient, client_link);
> +    for (link = client->channels; link != NULL; link = link->next) {
> +        rcc = link->data;
>          if (rcc->channel->type == type && rcc->channel->id == id) {
>              ret = rcc;
>              break;
> @@ -2157,12 +2160,11 @@ static RedChannelClient *red_client_get_channel(RedClient *client, int type, int
>  static void red_client_add_channel(RedClient *client, RedChannelClient *rcc)
>  {
>      spice_assert(rcc && client);
> -    ring_add(&client->channels, &rcc->client_link);
> +    client->channels = g_list_append(client->channels, rcc);
>      if (client->during_target_migrate && client->seamless_migrate) {
>          if (red_channel_client_set_migration_seamless(rcc))
>              client->num_migrated_channels++;
>      }
> -    client->channels_num++;
>  }
>  
>  MainChannelClient *red_client_get_main(RedClient *client) {
> @@ -2175,7 +2177,7 @@ void red_client_set_main(RedClient *client, MainChannelClient *mcc) {
>  
>  void red_client_semi_seamless_migrate_complete(RedClient *client)
>  {
> -    RingItem *link, *next;
> +    GList *link, *next;
>  
>      pthread_mutex_lock(&client->lock);
>      if (!client->during_target_migrate || client->seamless_migrate) {
> @@ -2184,12 +2186,15 @@ void red_client_semi_seamless_migrate_complete(RedClient *client)
>          return;
>      }
>      client->during_target_migrate = FALSE;
> -    RING_FOREACH_SAFE(link, next, &client->channels) {
> -        RedChannelClient *rcc = SPICE_CONTAINEROF(link, RedChannelClient, client_link);
> +    link = client->channels;
> +    while (link) {
> +        next = link->next;
> +        RedChannelClient *rcc = link->data;
>  
>          if (rcc->latency_monitor.timer) {
>              red_channel_client_start_ping_timer(rcc, PING_TEST_IDLE_NET_TIMEOUT_MS);
>          }
> +        link = next;
>      }
>      pthread_mutex_unlock(&client->lock);
>      reds_on_client_semi_seamless_migrate_complete(client->reds, client);
> diff --git a/server/red-channel.h b/server/red-channel.h
> index d4e7abe..7b4ec75 100644
> --- a/server/red-channel.h
> +++ b/server/red-channel.h
> @@ -237,8 +237,6 @@ typedef struct RedChannelClientConnectivityMonitor {
>  } RedChannelClientConnectivityMonitor;
>  
>  struct RedChannelClient {
> -    RingItem channel_link;
> -    RingItem client_link;
>      RedChannel *channel;
>      RedClient  *client;
>      RedsStream *stream;
> @@ -559,8 +557,7 @@ struct RedsState* red_channel_get_server(RedChannel *channel);
>  struct RedClient {
>      RedsState *reds;
>      RingItem link;
> -    Ring channels;
> -    int channels_num;
> +    GList *channels;
>      MainChannelClient *mcc;
>      pthread_mutex_t lock; // different channels can be in different threads
>  
> -- 
> 2.4.11
> 
> _______________________________________________
> 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: 819 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160510/155b5370/attachment.sig>