On Thu, Nov 12, 2015 at 4:36 PM, Frediano Ziglio <[email protected]> wrote: > >> On Thu, Nov 12, 2015 at 3:00 PM, Frediano Ziglio <[email protected]> wrote: >> > From: Marc-André Lureau <[email protected]> >> > >> > --- >> > 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 >> > [email protected] >> > 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.
Okay. So, let's skip the patches that are changing only changing the asserts (and try to split the assert changes from the other patches) at least till we have a final conclusion. > > Frediano _______________________________________________ Spice-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/spice-devel
