[Spice-devel] [PATCH 13/18] display: replace some dubious asserts

[Frediano Ziglio]

> On Mon, 2015-11-23 at 17:01 +0000, Frediano Ziglio wrote:
> > From: Marc-André Lureau <marcandre.lureau at gmail.com>
> > 
> > Reviewed-by: Jonathon Jongsma <jjongsma at redhat.com>
> 
> As I said in my previous review, I think that using g_return_if_fail() here
> is
> appropriate.
> 
> 
> 
> > ---
> >  server/display-channel.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/server/display-channel.c b/server/display-channel.c
> > index 5e75019..a178cc9 100644
> > --- a/server/display-channel.c
> > +++ b/server/display-channel.c
> > @@ -861,10 +861,10 @@ int
> > display_channel_wait_for_migrate_data(DisplayChannel
> > *display)
> >      RedChannelClient *rcc;
> >  
> >      spice_debug(NULL);
> > -    spice_assert(channel->clients_num == 1);
> > +    spice_warn_if_fail(channel->clients_num == 1);
> >  

Actually I was thinking. Here the client is the migration target?
I would hack this hunk too.

> >      rcc = SPICE_CONTAINEROF(ring_get_head(&channel->clients),
> > RedChannelClient, channel_link);
> > -    spice_assert(red_channel_client_waits_for_migrate_data(rcc));
> > +
> > spice_return_val_if_fail(red_channel_client_waits_for_migrate_data(rcc),
> > FALSE);
> >  

why not a

  if (!red_channel_client_waits_for_migrate_data(rcc)) {
      spice_warning("failed waiting for migration data");
      return FALSE;
  }

just to be sure.

> >      for (;;) {
> >          red_channel_client_receive(rcc);
> 

Frediano