Re: spice-server, logging and style

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Fri, Nov 20, 2015 at 01:12:27PM -0500, Frediano Ziglio wrote:
> assert (the base C) abort on condition failed and can be compiled out. Somebody
> by default disable these checks on release (I call it the Windows style) while
> other prefer to never disable (I call the Unix style). If disabled they are no
> op.
> 
> spice_assert (spice) abort on a condition failed and cannot be disabled.

I don't think we want to be able to disable assertions anyway, when they
are here it's because something bad would happen otherwise :)

> g_return_if_* (glib) return on a condition failed and cannot be disabled. If
> disabled they are no-op.

There is G_DISABLE_CHECKS to turn them into no-op.

> I think returning vs abortion here is a bit of a personal opinion. We should
> decide (kind polling) on what to do and stick to it. I personally prefer
> the abort if should be 100% true.
> This does not mean I'm suggesting to go all aborts or all returns, handling
> condition could mean to do stuff differently too.

Maybe you are suggesting more or less the same thing :).
To me, from worst to "less worse", when something unexpected happens:
- not detected, code continues running but behaves unpredictably (can
  easily lead to a security vulnerability if this can be triggered from
  the guest)
- detect the condition, and abort (assert())
- detect the condition, log it, and keep running (return_if_fail())

asserting is more comfortable for us developers, and probably easier,
but this also means we are killing a user VM, so this should not be done
lightly, which is why return_if_fail() is vastly better.
It's probably not always possible to easily deal gracefully with such
conditions, so yes, assert() is still an option when we don't have
better choices.

Christophe

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]