[Spice-devel,RFC] log: warn on spice_return*_if_fail instead of aborting

Submitted by Uri Lublin on Oct. 29, 2012, 9:46 a.m.

Details

Message ID 360d42f9e853184bcbeb26e0947d0fba3281cabc.1351503950.git.uril@redhat.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Uri Lublin Oct. 29, 2012, 9:46 a.m.
Currently log level for spice_return_if_fail and spice_return_val_if_failed
functions (#define macros really) is SPICE_LOG_LEVEL_CRITICAL.
By default spice abort level is SPICE_LOG_LEVEL_CRITICAL.
That means the program aborts upon a call to spice_return_if_fail
functions.

This patch changes the log level for spice_return_if_fail functions
to SPICE_LOG_LEVEL_WARNING, such that a warning message
is written to log, and the program continues.
---
 common/log.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/common/log.h b/common/log.h
index d9e6023..60a64de 100644
--- a/common/log.h
+++ b/common/log.h
@@ -58,7 +58,7 @@  void spice_log(const char *log_domain,
 #ifndef spice_return_if_fail
 #define spice_return_if_fail(x) SPICE_STMT_START {                      \
     if SPICE_LIKELY(x) { } else {                                       \
-        spice_log(SPICE_LOG_DOMAIN, SPICE_LOG_LEVEL_CRITICAL, SPICE_STRLOC, __FUNCTION__, "condition `%s' failed", #x); \
+        spice_log(SPICE_LOG_DOMAIN, SPICE_LOG_LEVEL_WARNING, SPICE_STRLOC, __FUNCTION__, "condition `%s' failed", #x); \
         return;                                                         \
     }                                                                   \
 } SPICE_STMT_END
@@ -67,7 +67,7 @@  void spice_log(const char *log_domain,
 #ifndef spice_return_val_if_fail
 #define spice_return_val_if_fail(x, val) SPICE_STMT_START {             \
     if SPICE_LIKELY(x) { } else {                                       \
-        spice_log(SPICE_LOG_DOMAIN, SPICE_LOG_LEVEL_CRITICAL, SPICE_STRLOC, __FUNCTION__, "condition `%s' failed", #x); \
+        spice_log(SPICE_LOG_DOMAIN, SPICE_LOG_LEVEL_WARNING, SPICE_STRLOC, __FUNCTION__, "condition `%s' failed", #x); \
         return (val);                                                   \
     }                                                                   \
 } SPICE_STMT_END

Comments

Hi

----- Mensaje original -----
> Currently log level for spice_return_if_fail and
> spice_return_val_if_failed
> functions (#define macros really) is SPICE_LOG_LEVEL_CRITICAL.
> By default spice abort level is SPICE_LOG_LEVEL_CRITICAL.
> That means the program aborts upon a call to spice_return_if_fail
> functions.
> 
> This patch changes the log level for spice_return_if_fail functions
> to SPICE_LOG_LEVEL_WARNING, such that a warning message
> is written to log, and the program continues.

This will differ from glib. The log level for g_return_if_fail is critical, but glib doesn't abort by default at this level, unless given G_DEBUG=fatal_criticals. The idea was to stay close to glib behaviour, in order to be easier to learn and easier to switch to glib eventually in the future.

Imho, the Spice server shouldn't crash on critical either. This can be tuned at compile time or at run-time with the SPICE_ABORT_LEVEL.

so small nack for the above reasons.
> Hi
> 
> ----- Mensaje original -----
> > Currently log level for spice_return_if_fail and
> > spice_return_val_if_failed
> > functions (#define macros really) is SPICE_LOG_LEVEL_CRITICAL.
> > By default spice abort level is SPICE_LOG_LEVEL_CRITICAL.
> > That means the program aborts upon a call to spice_return_if_fail
> > functions.
> > 
> > This patch changes the log level for spice_return_if_fail functions
> > to SPICE_LOG_LEVEL_WARNING, such that a warning message
> > is written to log, and the program continues.
> 
> This will differ from glib. The log level for g_return_if_fail is
> critical, but glib doesn't abort by default at this level, unless
> given G_DEBUG=fatal_criticals. The idea was to stay close to glib
> behaviour, in order to be easier to learn and easier to switch to
> glib eventually in the future.
> 
> Imho, the Spice server shouldn't crash on critical either. This can
> be tuned at compile time or at run-time with the SPICE_ABORT_LEVEL.
> 
> so small nack for the above reasons.

+1 to make critical not abort, explain that it is compatible to glib in commit message.

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