[Spice-devel] [PATCH] channel: Remove clients_num and use g_list_length

[Eduardo Lima (Etrunko)]

On 06/01/2016 11:58 AM, Frediano Ziglio wrote:
> clients_num was no more updated correctly.
> This fixes a regression introduced by
> 4028fb1c794ef2a212f22b024b1c39f460439e7b.
> The number of client is quite small so there is no much reason
> to cache the number of elements.
> 

Patch fixes the issue, but I think the commit message could be a bit
clearer by shuffling the phrases around.

What do you think about:

"""
This fixes a regression introduced by
4028fb1c794ef2a212f22b024b1c39f460439e7b, where clients_num was not
updated correctly anymore.

There is no reason to cache the number of elements on that list, as the
expected number of clients is quite small.
"""

No need to send another version, if you adjust the message.

Acked-by: Eduardo Lima (Etrunko) <etrunko at redhat.com>

> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  server/display-channel.c     | 2 +-
>  server/main-channel-client.c | 2 +-
>  server/main-channel.c        | 2 +-
>  server/red-channel.h         | 1 -
>  server/red-worker.c          | 2 +-
>  5 files changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/server/display-channel.c b/server/display-channel.c
> index f3bdb73..2888cad 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -319,7 +319,7 @@ static void pipes_add_drawable_after(DisplayChannel *display,
>          pipes_add_drawable(display, drawable);
>          return;
>      }
> -    if (num_other_linked != display->common.base.clients_num) {
> +    if (num_other_linked != g_list_length(display->common.base.clients)) {
>          GList *link, *next;
>          spice_debug("TODO: not O(n^2)");
>          FOREACH_CLIENT(display, link, next, dcc) {
> diff --git a/server/main-channel-client.c b/server/main-channel-client.c
> index 2188ebb..acf1bd4 100644
> --- a/server/main-channel-client.c
> +++ b/server/main-channel-client.c
> @@ -457,7 +457,7 @@ void main_channel_client_migrate_dst_complete(MainChannelClient *mcc)
>  {
>      if (mcc->mig_wait_prev_complete) {
>          if (mcc->mig_wait_prev_try_seamless) {
> -            spice_assert(mcc->base.channel->clients_num == 1);
> +            spice_assert(g_list_length(mcc->base.channel->clients) == 1);
>              red_channel_client_pipe_add_type(&mcc->base,
>                                               RED_PIPE_ITEM_TYPE_MAIN_MIGRATE_BEGIN_SEAMLESS);
>          } else {
> diff --git a/server/main-channel.c b/server/main-channel.c
> index 1a18200..42195e6 100644
> --- a/server/main-channel.c
> +++ b/server/main-channel.c
> @@ -102,7 +102,7 @@ static int main_channel_handle_migrate_data(RedChannelClient *rcc,
>      SpiceMigrateDataHeader *header = (SpiceMigrateDataHeader *)message;
>  
>      /* not supported with multi-clients */
> -    spice_assert(rcc->channel->clients_num == 1);
> +    spice_assert(g_list_length(rcc->channel->clients) == 1);
>  
>      if (size < sizeof(SpiceMigrateDataHeader) + sizeof(SpiceMigrateDataMain)) {
>          spice_printerr("bad message size %u", size);
> diff --git a/server/red-channel.h b/server/red-channel.h
> index 1907ae6..5b38f57 100644
> --- a/server/red-channel.h
> +++ b/server/red-channel.h
> @@ -306,7 +306,6 @@ struct RedChannel {
>      // Maybe replace these logic with ref count?
>      // TODO: rename to 'connected_clients'?
>      GList *clients;
> -    uint32_t clients_num;
>  
>      OutgoingHandlerInterface outgoing_cb;
>      IncomingHandlerInterface incoming_cb;
> diff --git a/server/red-worker.c b/server/red-worker.c
> index e99c9a1..a14f55d 100644
> --- a/server/red-worker.c
> +++ b/server/red-worker.c
> @@ -525,7 +525,7 @@ static void guest_set_client_capabilities(RedWorker *worker)
>          return;
>      }
>      if ((worker->display_channel == NULL) ||
> -        (RED_CHANNEL(worker->display_channel)->clients_num == 0)) {
> +        (RED_CHANNEL(worker->display_channel)->clients == NULL)) {
>          red_qxl_set_client_capabilities(worker->qxl, FALSE, caps);
>      } else {
>          // Take least common denominator
> 


-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etrunko at redhat.com