[Spice-devel] spice-server, logging and style

[Christophe Fergeau]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20151123/78441f4a/attachment-0001.sig>