[Spice-devel] [PATCH 1/7] worker: remove current_add assert

[Frediano Ziglio]

> On Thu, Nov 12, 2015 at 3:00 PM, Frediano Ziglio <fziglio at redhat.com> wrote:
> > From: Marc-André Lureau <marcandre.lureau at gmail.com>
> >
> > ---
> >  server/red_worker.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/server/red_worker.c b/server/red_worker.c
> > index 6bc015f..d33a352 100644
> > --- a/server/red_worker.c
> > +++ b/server/red_worker.c
> > @@ -2254,7 +2254,7 @@ static inline int current_add(RedWorker *worker, Ring
> > *ring, Drawable *drawable)
> >      QRegion exclude_rgn;
> >      RingItem *exclude_base = NULL;
> >
> > -    spice_assert(!region_is_empty(&item->base.rgn));
> > +    spice_return_val_if_fail(!region_is_empty(&item->base.rgn), FALSE);
> >      region_init(&exclude_rgn);
> >      now = ring_next(ring, ring);
> >
> > --
> > 2.4.3
> >
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/spice-devel
> 
> ACK!
> 

On this and 4/7 (asserts and so on)

There has been some discussion going on (see "spice-server, logging and style"
on ML)
- me: I'd like to have some reasoning and comments and not changed silently
  (this is not hidden but there is no much reasoning).
  I don't want to continue on memory errors but stop (ie: spice-assert);
- Marc: prefer to not have assert so to not stop spice-server. I don't agree
  for memory errors;
- Marc, Christophe: like to have logs. I pointed out that too log cause DoS,
  Christophe sent a link that Qemu guys are worried of logs too;
- Pavel: pointed out that some of these hidden changes are not good (don't
  remember exactly);
- spice-server mainly reflect glib macro beside critical is fatal causing
  return_if macro to abort;
- somebody suggested to use glib macro directly. But they are not 100% compatible
  so we decided to not do it now. However Pavel (and I agree with him) does
  not like to do a change that we need to review later (that is: let's doing once
  in the right ways);
- Uri is mainly (this is a big sum up!) suggesting to use spice-assert for
  internal conditions and something not aborting for others. This patch is against
  this rule as item are created by spice-server.

Surely I have forgot something or blame somebody for stuff others said.

I think is better to move these changes to later time when we have more
clearer idea. I'm not sure however we all agree so I posted the patches anyway.

Frediano