[Spice-devel] [PATCH 5/7] Don't increment num_clients_mig_wait twice

[Jonathon Jongsma]

On Wed, 2016-10-12 at 06:57 -0400, Frediano Ziglio wrote:
> > 
> > 
> > When MainChannelClient was split to a separate file, the
> > responsibility
> > for incrementing this field was supposed to belong to the
> > MainChannel
> > function (main_channel_connect_semi_seamless()), but by mistake it
> > was
> > incremented both there and in the client function
> > (main_channel_client_connect_semi_seamless()).
> > 
> > The bug was introduced in a11b785f191d2381932f8c1bb6281539f2afe483
> > ---
> >  server/main-channel-client.c | 2 --
> >  1 file changed, 2 deletions(-)
> > 
> > diff --git a/server/main-channel-client.c b/server/main-channel-
> > client.c
> > index b30083f..0913028 100644
> > --- a/server/main-channel-client.c
> > +++ b/server/main-channel-client.c
> > @@ -704,7 +704,6 @@ void
> > main_channel_client_migrate(RedChannelClient *rcc)
> >  gboolean
> > main_channel_client_connect_semi_seamless(MainChannelClient *mcc)
> >  {
> >      RedChannelClient *rcc = RED_CHANNEL_CLIENT(mcc);
> > -    MainChannel* main_channel =
> > MAIN_CHANNEL(red_channel_client_get_channel(rcc));
> >      if (red_channel_client_test_remote_cap(rcc,
> >                                             SPICE_MAIN_CAP_SEMI_SEA
> > MLESS_MIGRATE))
> >                                             {
> >          RedClient *client = red_channel_client_get_client(rcc);
> > @@ -718,7 +717,6 @@ gboolean
> > main_channel_client_connect_semi_seamless(MainChannelClient *mcc)
> >              mcc->priv->mig_wait_connect = TRUE;
> >          }
> >          mcc->priv->mig_connect_ok = FALSE;
> > -        main_channel->num_clients_mig_wait++;
> >          return TRUE;
> >      }
> >      return FALSE;
> 
> Acked-by: Frediano Ziglio <fziglio at redhat.com>
> 
> I think we have a lack of testing, at least fast ones.

Yes, definitely.

> How did you discover this? 

Just code inspection. While splitting out the MainChannelPrivate stuff
into a separate patch, I noticed that this hunk was included in the
"Convert RedChannel Hierarchy to GObject" patch. And it looked odd to
me, so I went to look at the history of this particular code.

> Did you manage to reproduce the issue?

No.

> 
> Frediano