[Spice-devel] [PATCH spice-common 1/4] log: replace spice log with glib

Marc-André Lureau marcandre.lureau at redhat.com
Tue Jun 13 09:42:07 UTC 2017


Hi

----- Original Message -----
> On Mon, 2017-06-12 at 12:19 +0400, marcandre.lureau at redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau at redhat.com>
> > 
> > Remove all the deprecated spice log macro/environment variables
> > facility in favour of glib. The following patches will bring some new
> > log macro with "category" and structured logging.
> 
> This patch does quite a bit more than you suggest here and should be
> expanded. In particular, it:
>  - logs debug messages to G_LOG_DOMAIN instead of SPICE_LOG_DOMAIN
>  - treats SPICE_LOG_DOMAIN as a subdomain and prepends it to the
> message
>  - Removes the spice_log() function itself
>  - Removes the file location and function name formatting from the log
> message
>  - Removes a bunch of test stuff
> 
> More comments below.

This is an RFC, so I didn't explain the details, but I will use your comment, thanks :)

> 
> > 
> > Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>
> > ---
> >  common/log.c         | 177 -----------------------------
> >  common/log.h         | 121 ++++++++------------
> >  tests/test-logging.c | 316 +--------------------------------------
> > ------------
> >  3 files changed, 52 insertions(+), 562 deletions(-)
> > 
> > diff --git a/common/log.c b/common/log.c
> > index 92f5bc0..6a15a30 100644
> > --- a/common/log.c
> > +++ b/common/log.c
> > @@ -18,180 +18,3 @@
> >  #ifdef HAVE_CONFIG_H
> >  #include <config.h>
> >  #endif
> > -
> > -#include <glib.h>
> > -#include <stdlib.h>
> > -#include <stdio.h>
> > -#include <sys/types.h>
> > -#ifndef _MSC_VER
> > -#include <unistd.h>
> > -#endif
> > -
> > -#include "log.h"
> > -#include "backtrace.h"
> > -
> > -static int glib_debug_level = INT_MAX;
> > -static int abort_mask = 0;
> > -
> > -#ifndef SPICE_ABORT_MASK_DEFAULT
> > -#ifdef SPICE_DISABLE_ABORT
> > -#define SPICE_ABORT_MASK_DEFAULT 0
> > -#else
> > -#define SPICE_ABORT_MASK_DEFAULT
> > (G_LOG_LEVEL_CRITICAL|G_LOG_LEVEL_ERROR)
> > -#endif
> > -#endif
> > -
> > -typedef enum {
> > -    SPICE_LOG_LEVEL_ERROR,
> > -    SPICE_LOG_LEVEL_CRITICAL,
> > -    SPICE_LOG_LEVEL_WARNING,
> > -    SPICE_LOG_LEVEL_INFO,
> > -    SPICE_LOG_LEVEL_DEBUG,
> > -} SpiceLogLevel;
> > -
> > -static GLogLevelFlags spice_log_level_to_glib(SpiceLogLevel level)
> > -{
> > -    static const GLogLevelFlags glib_levels[] = {
> > -        [ SPICE_LOG_LEVEL_ERROR ] = G_LOG_LEVEL_ERROR,
> > -        [ SPICE_LOG_LEVEL_CRITICAL ] = G_LOG_LEVEL_CRITICAL,
> > -        [ SPICE_LOG_LEVEL_WARNING ] = G_LOG_LEVEL_WARNING,
> > -        [ SPICE_LOG_LEVEL_INFO ] = G_LOG_LEVEL_INFO,
> > -        [ SPICE_LOG_LEVEL_DEBUG ] = G_LOG_LEVEL_DEBUG,
> > -    };
> > -    g_return_val_if_fail (level >= 0, G_LOG_LEVEL_ERROR);
> > -    g_return_val_if_fail (level < G_N_ELEMENTS(glib_levels),
> > G_LOG_LEVEL_DEBUG);
> > -
> > -    return glib_levels[level];
> > -}
> > -
> > -static void spice_log_set_debug_level(void)
> > -{
> > -    if (glib_debug_level == INT_MAX) {
> > -        const char *debug_str = g_getenv("SPICE_DEBUG_LEVEL");
> > -        if (debug_str != NULL) {
> > -            int debug_level;
> > -            char *debug_env;
> > -
> > -            /* FIXME: To be removed after enough deprecation time */
> > -            g_warning("Setting SPICE_DEBUG_LEVEL is deprecated, use
> > G_MESSAGES_DEBUG instead");
> > -            debug_level = atoi(debug_str);
> > -            if (debug_level > SPICE_LOG_LEVEL_DEBUG) {
> > -                debug_level = SPICE_LOG_LEVEL_DEBUG;
> > -            }
> > -            glib_debug_level = spice_log_level_to_glib(debug_level);
> > -
> > -            /* If the debug level is too high, make sure we don't
> > try to enable
> > -             * display of glib debug logs */
> > -            if (debug_level < SPICE_LOG_LEVEL_INFO)
> > -                return;
> > -
> > -            /* Make sure GLib default log handler will show the
> > debug messages. Messing with
> > -             * environment variables like this is ugly, but this
> > only happens when the legacy
> > -             * SPICE_DEBUG_LEVEL is used
> > -             */
> > -            debug_env = (char *)g_getenv("G_MESSAGES_DEBUG");
> > -            if (debug_env == NULL) {
> > -                g_setenv("G_MESSAGES_DEBUG", SPICE_LOG_DOMAIN,
> > FALSE);
> > -            } else {
> > -                debug_env = g_strconcat(debug_env, " ",
> > SPICE_LOG_DOMAIN, NULL);
> > -                g_setenv("G_MESSAGES_DEBUG", SPICE_LOG_DOMAIN,
> > FALSE);
> > -                g_free(debug_env);
> > -            }
> > -        }
> > -    }
> > -}
> > -
> > -static void spice_log_set_abort_level(void)
> > -{
> > -    if (abort_mask == 0) {
> > -        const char *abort_str = g_getenv("SPICE_ABORT_LEVEL");
> > -        if (abort_str != NULL) {
> > -            GLogLevelFlags glib_abort_level;
> > -
> > -            /* FIXME: To be removed after enough deprecation time */
> > -            g_warning("Setting SPICE_ABORT_LEVEL is deprecated, use
> > G_DEBUG instead");
> > -            glib_abort_level =
> > spice_log_level_to_glib(atoi(abort_str));
> > -            unsigned int fatal_mask = G_LOG_FATAL_MASK;
> > -            while (glib_abort_level >= G_LOG_LEVEL_ERROR) {
> > -                fatal_mask |= glib_abort_level;
> > -                glib_abort_level >>= 1;
> > -            }
> > -            abort_mask = fatal_mask;
> > -            g_log_set_fatal_mask(SPICE_LOG_DOMAIN, fatal_mask);
> > -        } else {
> > -            abort_mask = SPICE_ABORT_MASK_DEFAULT;
> > -        }
> > -    }
> > -}
> > -
> > -static void spice_logger(const gchar *log_domain,
> > -                         GLogLevelFlags log_level,
> > -                         const gchar *message,
> > -                         gpointer user_data G_GNUC_UNUSED)
> > -{
> > -    if ((log_level & G_LOG_LEVEL_MASK) > glib_debug_level) {
> > -        return; // do not print anything
> > -    }
> > -    g_log_default_handler(log_domain, log_level, message, NULL);
> > -}
> > -
> > -SPICE_CONSTRUCTOR_FUNC(spice_log_init)
> > -{
> > -
> > -    spice_log_set_debug_level();
> > -    spice_log_set_abort_level();
> > -    g_log_set_handler(SPICE_LOG_DOMAIN,
> > -                      G_LOG_LEVEL_MASK | G_LOG_FLAG_FATAL |
> > G_LOG_FLAG_RECURSION,
> > -                      spice_logger, NULL);
> > -    /* Threading is always enabled from 2.31.0 onwards */
> > -    /* Our logging is potentially used from different threads.
> > -     * Older glibs require that g_thread_init() is called when
> > -     * doing that. */
> > -#if !GLIB_CHECK_VERSION(2, 31, 0)
> > -    if (!g_thread_supported())
> > -        g_thread_init(NULL);
> > -#endif
> > -}
> > -
> > -static void spice_logv(const char *log_domain,
> > -                       GLogLevelFlags log_level,
> > -                       const char *strloc,
> > -                       const char *function,
> > -                       const char *format,
> > -                       va_list args)
> > -{
> > -    GString *log_msg;
> > -
> > -    if ((log_level & G_LOG_LEVEL_MASK) > glib_debug_level) {
> > -        return; // do not print anything
> > -    }
> > -
> > -    log_msg = g_string_new(NULL);
> > -    if (strloc && function) {
> > -        g_string_append_printf(log_msg, "%s:%s: ", strloc,
> > function);
> > -    }
> > -    if (format) {
> > -        g_string_append_vprintf(log_msg, format, args);
> > -    }
> > -    g_log(log_domain, log_level, "%s", log_msg->str);
> > -    g_string_free(log_msg, TRUE);
> > -
> > -    if ((abort_mask & log_level) != 0) {
> > -        spice_backtrace();
> > -        abort();
> > -    }
> > -}
> > -
> > -void spice_log(const char *log_domain,
> > -               GLogLevelFlags log_level,
> > -               const char *strloc,
> > -               const char *function,
> > -               const char *format,
> > -               ...)
> > -{
> > -    va_list args;
> > -
> > -    va_start (args, format);
> > -    spice_logv (log_domain, log_level, strloc, function, format,
> > args);
> > -    va_end (args);
> > -}
> > diff --git a/common/log.h b/common/log.h
> > index 1ea2313..b86fd08 100644
> > --- a/common/log.h
> > +++ b/common/log.h
> > @@ -18,87 +18,58 @@
> >  #ifndef H_SPICE_LOG
> >  #define H_SPICE_LOG
> >  
> > -#include <stdarg.h>
> > -#include <stdio.h>
> > -#include <glib.h>
> > -#include <spice/macros.h>
> > +#ifndef G_LOG_DOMAIN
> > +#define G_LOG_DOMAIN "Spice"
> > +#endif
> 
> 
> I'm not sure that it's a good idea to set a default domain here. I
> think we should just emit an error if it's not defined. spice-server
> and spice-gtk use different domains, so I think it's reasonable to
> leave this definition up to the users of spice-common.

We are actually converging to have only "Spice" glog domain. Later, we might be able to remove the #ifndef, then it will error out if there is another domain defined.

> 
> >  
> > +#include <glib.h>
> > +#include <stdbool.h>
> >  #include "macros.h"
> >  
> > -SPICE_BEGIN_DECLS
> > +G_BEGIN_DECLS
> >  
> > +/* deprecated, please use glib directly */
> > +#define spice_return_if_fail(x) g_return_if_fail(x)
> > +#define spice_return_val_if_fail(x, val) g_return_val_if_fail(x,
> > val)
> > +#define spice_warn_if_reached() g_warn_if_reached()
> > +#define spice_warn_if_fail(x) g_warn_if_fail(x)
> > +#define spice_assert(x) g_assert(x)
> > +#define spice_static_assert(x) G_STATIC_ASSERT(x)
> > +
> > +/* deprecated */
> >  #ifndef SPICE_LOG_DOMAIN
> > -#define SPICE_LOG_DOMAIN "Spice"
> > +#define SPICE_LOG_DOMAIN __FILE__
> >  #endif
> 
> There's a good argument to be made that a file name is a useful default
> categorization for logging. But there are good reasons against it too,
> I think. The filenames become a sort of "developer API". For example,
> if you are bisecting spice-gtk and a file gets renamed or some code
> gets moved from one file to another, you'll have to adjust your
> debugging specification as you move through the commits. The same thing
> is true if you ever have to ask for a targeted log from a customer or
> QA, etc. Of course, you could enable all categories, but that may not
> be very desirable. If you have categories that are explicitly defined
> (e.g. "input", "display", etc), you'll be able to expect that they'll
> continue working the same regardless of where the code is located or
> what the files are named.
> 


Good points about filename & category name.

Remember this code is deprecated and there for the transition period. I wanted to keep the existing SPICE_LOG_DOMAIN in the log if it's defined, but have a sensible default.

Once the switch to category API, this will go away.

> >  
> > -#define SPICE_STRLOC  __FILE__ ":" G_STRINGIFY (__LINE__)
> > -
> > -void spice_log(const char *log_domain,
> > -               GLogLevelFlags log_level,
> > -               const char *strloc,
> > -               const char *function,
> > -               const char *format,
> > -               ...) SPICE_ATTR_PRINTF(5, 6);
> > -
> > -#define spice_return_if_fail(x) G_STMT_START
> > {                          \
> > -    if G_LIKELY(x) { } else
> > {                                           \
> > -        spice_log(SPICE_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL,
> > SPICE_STRLOC, G_STRFUNC, "condition `%s' failed", #x); \
> > -        return;
> >     \
> > -    }
> >     \
> > -} G_STMT_END
> > -
> > -#define spice_return_val_if_fail(x, val) G_STMT_START
> > {                 \
> > -    if G_LIKELY(x) { } else
> > {                                           \
> > -        spice_log(SPICE_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL,
> > SPICE_STRLOC, __FUNCTION__, "condition `%s' failed", #x); \
> > -        return
> > (val);                                                   \
> > -    }
> >     \
> > -} G_STMT_END
> > -
> > -#define spice_warn_if_reached() G_STMT_START
> > {                          \
> > -    spice_log(SPICE_LOG_DOMAIN, G_LOG_LEVEL_WARNING, SPICE_STRLOC,
> > __FUNCTION__, "should not be reached"); \
> > -} G_STMT_END
> > -
> > -#define spice_printerr(format, ...) G_STMT_START
> > {                      \
> > -    fprintf(stderr, "%s: " format "\n", __FUNCTION__, ##
> > __VA_ARGS__);  \
> > -} G_STMT_END
> > -
> > -#define spice_info(format, ...) G_STMT_START
> > {                         \
> > -    spice_log(SPICE_LOG_DOMAIN, G_LOG_LEVEL_INFO, SPICE_STRLOC,
> > __FUNCTION__, format, ## __VA_ARGS__); \
> > -} G_STMT_END
> > -
> > -#define spice_debug(format, ...) G_STMT_START
> > {                         \
> > -    spice_log(SPICE_LOG_DOMAIN, G_LOG_LEVEL_DEBUG, SPICE_STRLOC,
> > __FUNCTION__, format, ## __VA_ARGS__); \
> > -} G_STMT_END
> > -
> > -#define spice_warning(format, ...) G_STMT_START
> > {                       \
> > -    spice_log(SPICE_LOG_DOMAIN, G_LOG_LEVEL_WARNING, SPICE_STRLOC,
> > __FUNCTION__, format, ## __VA_ARGS__); \
> > -} G_STMT_END
> > -
> > -#define spice_critical(format, ...) G_STMT_START
> > {                          \
> > -    spice_log(SPICE_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL, SPICE_STRLOC,
> > __FUNCTION__, format, ## __VA_ARGS__); \
> > -} G_STMT_END
> > -
> > -#define spice_error(format, ...) G_STMT_START
> > {                         \
> > -    spice_log(SPICE_LOG_DOMAIN, G_LOG_LEVEL_ERROR, SPICE_STRLOC,
> > __FUNCTION__, format, ## __VA_ARGS__); \
> > -} G_STMT_END
> > -
> > -#define spice_warn_if_fail(x) G_STMT_START {            \
> > -    if G_LIKELY(x) { } else {                           \
> > -        spice_warning("condition `%s' failed", #x);     \
> > -    }                                                   \
> > -} G_STMT_END
> > -
> > -#define spice_assert(x) G_STMT_START {                  \
> > -    if G_LIKELY(x) { } else {                           \
> > -        spice_error("assertion `%s' failed", #x);       \
> > -    }                                                   \
> > -} G_STMT_END
> > -
> > -/* FIXME: improve that some day.. */
> > -#define spice_static_assert(x) SPICE_STMT_START {       \
> > -    spice_assert(x);                                    \
> > -} SPICE_STMT_END
> > -
> > -SPICE_END_DECLS
> > +#define spice_printerr(format, ...) g_printerr(SPICE_LOG_DOMAIN ": "
> > format, ## __VA_ARGS__)
> > +#define spice_info(format, ...) g_info(SPICE_LOG_DOMAIN ": " format,
> > ## __VA_ARGS__)
> > +#define spice_warning(format, ...) g_warning(SPICE_LOG_DOMAIN ": "
> > format, ## __VA_ARGS__)
> > +#define spice_critical(format, ...) g_critical(SPICE_LOG_DOMAIN ": "
> > format, ## __VA_ARGS__)
> > +#define spice_error(format, ...) g_error(SPICE_LOG_DOMAIN ": "
> > format, ## __VA_ARGS__)
> > +
> > +/* because spice_debug(NULL) exists.. */
> > +G_GNUC_PRINTF(5, 6)
> > +static inline void _spice_debug(const char *domain, const char
> > *file,
> > +                                const char *line, const char *func,
> > +                                const char *msg, ...)
> > +{
> > +    va_list args;
> > +    GString *s = g_string_new(NULL);
> > +
> > +    va_start(args, msg);
> > +    g_string_printf(s, "%s: ", domain);
> > +    if (msg) {
> > +        g_string_append_vprintf(s, msg, args);
> > +    }
> > +    g_debug("%s", s->str);
> > +    va_end(args);
> > +    g_string_free(s, true);
> > +}
> 
> For what it's worth, Frediano had just committed a patch
> (564635e3c1a53390c9748bfb70d4129b411820e7) which attempts to avoid
> string formatting operation when logging is not enabled. This patch
> adds some unconditional string formatting.
> 

Same argument, this is temporary. As soon as spice_debug(NULL) goes away, this will too.

I can't wait for Christophe F. to send the patch :)

> > +
> > +#define spice_debug(msg, ...) \
> > +    _spice_debug(SPICE_LOG_DOMAIN, __FILE__, G_STRINGIFY(__LINE__),
> > \
> > +                 G_STRFUNC, msg, ## __VA_ARGS__)
> > +
> > +G_END_DECLS
> >  
> >  #endif /* H_SPICE_LOG */
> > diff --git a/tests/test-logging.c b/tests/test-logging.c
> > index f1ad1b6..03f367a 100644
> > --- a/tests/test-logging.c
> > +++ b/tests/test-logging.c
> > @@ -18,301 +18,8 @@
> >  #include <config.h>
> >  #endif
> >  
> > -#define G_LOG_DOMAIN "Spice"
> > -#define SPICE_LOG_DOMAIN G_LOG_DOMAIN
> > -
> > -#include <glib.h>
> > -#include <stdlib.h>
> > -
> >  #include "common/log.h"
> > -
> > -#define OTHER_LOG_DOMAIN "Other"
> > -#define LOG_OTHER_HELPER(suffix,
> > level)                                          \
> > -    static void G_PASTE(other_, suffix)(const gchar *format,
> > ...)                \
> > -    {
> >              \
> > -        va_list
> > args;                                                            \
> > -
> >              \
> > -        va_start (args,
> > format);                                                 \
> > -        g_logv(OTHER_LOG_DOMAIN, G_PASTE(G_LOG_LEVEL_, level),
> > format, args);    \
> > -        va_end
> > (args);                                                           \
> > -    }
> > -
> > -/* Set of helpers to log in a different log domain than "Spice" */
> > -LOG_OTHER_HELPER(debug, DEBUG)
> > -LOG_OTHER_HELPER(info, INFO)
> > -LOG_OTHER_HELPER(message, MESSAGE)
> > -LOG_OTHER_HELPER(warning, WARNING)
> > -LOG_OTHER_HELPER(critical, CRITICAL)
> > -
> >  #if GLIB_CHECK_VERSION(2,38,0)
> > -/* Checks that spice_warning() aborts after changing
> > SPICE_ABORT_LEVEL */
> > -static void test_spice_abort_level(void)
> > -{
> > -    if (g_test_subprocess()) {
> > -        spice_warning("spice_warning");
> > -        return;
> > -    }
> > -    /* 2 = SPICE_LOG_LEVEL_WARNING  */
> > -    g_setenv("SPICE_ABORT_LEVEL", "2", TRUE);
> > -    g_test_trap_subprocess(NULL, 0, 0);
> > -    g_unsetenv("SPICE_ABORT_LEVEL");
> > -    g_test_trap_assert_failed();
> > -    g_test_trap_assert_stderr("*SPICE_ABORT_LEVEL*deprecated*");
> > -    g_test_trap_assert_stderr("*spice_warning*");
> > -}
> > -
> > -/* Checks that g_warning() aborts after changing SPICE_ABORT_LEVEL
> > */
> > -static void test_spice_abort_level_g_warning(void)
> > -{
> > -    if (g_test_subprocess()) {
> > -        g_warning("g_warning");
> > -        return;
> > -    }
> > -    g_setenv("SPICE_ABORT_LEVEL", "2", TRUE);
> > -    g_test_trap_subprocess(NULL, 0, 0);
> > -    g_unsetenv("SPICE_ABORT_LEVEL");
> > -    g_test_trap_assert_failed();
> > -    g_test_trap_assert_stderr("*SPICE_ABORT_LEVEL*deprecated*");
> > -    g_test_trap_assert_stderr("*g_warning*");
> > -}
> > -
> > -/* Checks that spice_warning() aborts after setting G_DEBUG=fatal-
> > warnings */
> > -static void test_spice_fatal_warning(void)
> > -{
> > -    g_setenv("G_DEBUG", "fatal-warnings", TRUE);
> > -    if (g_test_subprocess()) {
> > -        spice_warning("spice_warning");
> > -        return;
> > -    }
> > -    g_test_trap_subprocess(NULL, 0, 0);
> > -    g_test_trap_assert_failed();
> > -    g_test_trap_assert_stderr("*spice_warning*");
> > -    g_unsetenv("G_DEBUG");
> > -}
> > -
> > -/* Checks that spice_critical() aborts by default if
> > SPICE_DISABLE_ABORT is not
> > - * defined at compile-time */
> > -static void test_spice_fatal_critical(void)
> > -{
> > -    if (g_test_subprocess()) {
> > -        spice_critical("spice_critical");
> > -        return;
> > -    }
> > -    g_test_trap_subprocess(NULL, 0, 0);
> > -#ifdef SPICE_DISABLE_ABORT
> > -    g_test_trap_assert_passed();
> > -#else
> > -    g_test_trap_assert_failed();
> > -#endif
> > -    g_test_trap_assert_stderr("*spice_critical*");
> > -}
> > -
> > -/* Checks that g_critical() does not abort by default */
> > -static void test_spice_non_fatal_g_critical(void)
> > -{
> > -    if (g_test_subprocess()) {
> > -        g_critical("g_critical");
> > -        return;
> > -    }
> > -    g_test_trap_subprocess(NULL, 0, 0);
> > -    g_test_trap_assert_passed();
> > -    g_test_trap_assert_stderr("*g_critical*");
> > -}
> > -
> > -/* Checks that g_critical() aborts after setting G_DEBUG=fatal-
> > criticals */
> > -static void test_spice_fatal_g_critical(void)
> > -{
> > -    g_setenv("G_DEBUG", "fatal-criticals", TRUE);
> > -    if (g_test_subprocess()) {
> > -        g_critical("g_critical");
> > -        return;
> > -    }
> > -    g_test_trap_subprocess(NULL, 0, 0);
> > -    g_test_trap_assert_failed();
> > -    g_test_trap_assert_stderr("*g_critical*");
> > -    g_unsetenv("G_DEBUG");
> > -}
> > -
> > -/* Checks that spice_return_if_fail() aborts by default unless
> > - * SPICE_DISABLE_ABORT was defined at compile time*/
> > -static void test_spice_fatal_return_if_fail(void)
> > -{
> > -    if (g_test_subprocess()) {
> > -        spice_return_if_fail(FALSE);
> > -        return;
> > -    }
> > -    g_test_trap_subprocess(NULL, 0, 0);
> > -#ifdef SPICE_DISABLE_ABORT
> > -    g_test_trap_assert_passed();
> > -#else
> > -    g_test_trap_assert_failed();
> > -#endif
> > -    g_test_trap_assert_stderr("*test_spice_fatal_return_if_fail*");
> > -}
> > -
> > -/* Checks that g_return_if_fail() does not abort by default */
> > -static void test_spice_non_fatal_g_return_if_fail(void)
> > -{
> > -    if (g_test_subprocess()) {
> > -        g_return_if_fail(FALSE);
> > -        return;
> > -    }
> > -    g_test_trap_subprocess(NULL, 0, 0);
> > -    g_test_trap_assert_passed();
> > -}
> > -
> > -/* Checks that spice_*, g_* and other_* (different log domain as
> > g_*) all
> > - * go through g_log() with the correct domain/log level. This then
> > checks
> > - * that only logs with level 'message' or higher are shown by
> > default.
> > - */
> > -static void test_log_levels(void)
> > -{
> > -    if (g_test_subprocess()) {
> > -        g_test_expect_message(SPICE_LOG_DOMAIN,
> > -                              G_LOG_LEVEL_WARNING,
> > -                              "*spice_warning");
> > -        spice_warning("spice_warning");
> > -        g_test_expect_message(SPICE_LOG_DOMAIN,
> > -                              G_LOG_LEVEL_INFO,
> > -                              "*spice_info");
> > -        spice_info("spice_info");
> > -        g_test_expect_message(SPICE_LOG_DOMAIN,
> > -                              G_LOG_LEVEL_DEBUG,
> > -                              "*spice_debug");
> > -        spice_debug("spice_debug");
> > -
> > -        g_test_expect_message(G_LOG_DOMAIN,
> > -                              G_LOG_LEVEL_CRITICAL,
> > -                              "*g_critical");
> > -        g_critical("g_critical");
> > -        g_test_expect_message(G_LOG_DOMAIN,
> > -                              G_LOG_LEVEL_WARNING,
> > -                              "*g_warning");
> > -        g_warning("g_warning");
> > -        g_test_expect_message(G_LOG_DOMAIN,
> > -                              G_LOG_LEVEL_MESSAGE,
> > -                              "*g_message");
> > -        g_message("g_message");
> > -        g_test_expect_message(G_LOG_DOMAIN,
> > -                              G_LOG_LEVEL_INFO,
> > -                              "*g_info");
> > -        g_info("g_info");
> > -        g_test_expect_message(G_LOG_DOMAIN,
> > -                              G_LOG_LEVEL_DEBUG,
> > -                              "*g_debug");
> > -        g_debug("g_debug");
> > -
> > -        g_test_expect_message(OTHER_LOG_DOMAIN,
> > -                              G_LOG_LEVEL_CRITICAL,
> > -                              "*other_critical");
> > -        other_critical("other_critical");
> > -        g_test_expect_message(OTHER_LOG_DOMAIN,
> > -                              G_LOG_LEVEL_WARNING,
> > -                              "*other_warning");
> > -        other_warning("other_warning");
> > -        g_test_expect_message(OTHER_LOG_DOMAIN,
> > -                              G_LOG_LEVEL_MESSAGE,
> > -                              "*other_message");
> > -        other_message("other_message");
> > -        g_test_expect_message(OTHER_LOG_DOMAIN,
> > -                              G_LOG_LEVEL_INFO,
> > -                              "*other_info");
> > -        other_info("other_info");
> > -        g_test_expect_message(OTHER_LOG_DOMAIN,
> > -                              G_LOG_LEVEL_DEBUG,
> > -                              "*other_debug");
> > -        other_debug("other_debug");
> > -
> > -        g_test_assert_expected_messages();
> > -
> > -
> > -        /* g_test_expected_message only checks whether the
> > appropriate messages got up to g_log()
> > -         * The following calls will be caught by the parent process
> > to check what was (not) printed
> > -         * to stdout/stderr
> > -         */
> > -        spice_warning("spice_warning");
> > -        spice_info("spice_info");
> > -        spice_debug("spice_debug");
> > -
> > -        g_critical("g_critical");
> > -        g_warning("g_warning");
> > -        g_message("g_message");
> > -        g_info("g_info");
> > -        g_debug("g_debug");
> > -
> > -        other_critical("other_critical");
> > -        other_warning("other_warning");
> > -        other_message("other_message");
> > -        other_info("other_info");
> > -        other_debug("other_debug");
> > -
> > -        return;
> > -    }
> > -    g_test_trap_subprocess(NULL, 0, 0);
> > -    g_test_trap_assert_passed();
> > -    g_test_trap_assert_stderr("*spice_warning\n*g_critical\n*g_warni
> > ng\n*g_message\n*other_critical\n*other_warning\n*other_message\n");
> > -}
> > -
> > -/* Checks that SPICE_DEBUG_LEVEL impacts spice_debug(), g_debug()
> > but not other_debug() */
> > -static void test_spice_debug_level(void)
> > -{
> > -    if (g_test_subprocess()) {
> > -        /* g_test_expected_message only checks whether the
> > appropriate messages got up to g_log()
> > -         * The following calls will be caught by the parent process
> > to check what was (not) printed
> > -         * to stdout/stderr
> > -         */
> > -        spice_info("spice_info");
> > -        g_debug("g_debug");
> > -        spice_debug("spice_debug");
> > -        other_debug("other_debug");
> > -
> > -        return;
> > -    }
> > -
> > -    g_unsetenv("G_MESSAGES_DEBUG");
> > -    g_setenv("SPICE_DEBUG_LEVEL", "5", TRUE);
> > -    g_test_trap_subprocess(NULL, 0, 0);
> > -    g_unsetenv("SPICE_DEBUG_LEVEL");
> > -    g_test_trap_assert_passed();
> > -    g_test_trap_assert_stderr("*SPICE_DEBUG_LEVEL*deprecated*");
> > -    g_test_trap_assert_stdout("*spice_info\n*g_debug\n*spice_debug\n
> > ");
> > -}
> > -
> > -/* Checks that raising SPICE_DEBUG_LEVEL allows to only show
> > spice_warning() and spice_critical()
> > - * messages, as well as g_warning() and g_critical(), but does not
> > impact other_message()
> > - */
> > -static void test_spice_debug_level_warning(void)
> > -{
> > -    if (g_test_subprocess()) {
> > -        spice_info("spice_info");
> > -        spice_debug("spice_debug");
> > -        spice_warning("spice_warning");
> > -        spice_critical("spice_critical");
> > -        g_debug("g_debug");
> > -        g_info("g_info");
> > -        g_message("g_message");
> > -        g_warning("g_warning");
> > -        g_critical("g_critical");
> > -        other_debug("other_debug");
> > -        other_info("other_info");
> > -        other_message("other_message");
> > -        other_warning("other_warning");
> > -        other_critical("other_critical");
> > -
> > -        return;
> > -    }
> > -
> > -    g_setenv("SPICE_ABORT_LEVEL", "0", TRUE);
> > -    g_setenv("SPICE_DEBUG_LEVEL", "1", TRUE);
> > -    g_test_trap_subprocess(NULL, 0, 0);
> > -    g_unsetenv("SPICE_ABORT_LEVEL");
> > -    g_unsetenv("SPICE_DEBUG_LEVEL");
> > -    g_test_trap_assert_passed();
> > -    g_test_trap_assert_stderr("*SPICE_DEBUG_LEVEL*deprecated*");
> > -    g_test_trap_assert_stderr("*SPICE_ABORT_LEVEL*deprecated*");
> > -    g_test_trap_assert_stderr("*spice_critical\n*g_critical\n*other_
> > message\n*other_warning\n*other_critical\n");
> > -}
> > -
> >  /* Checks that setting G_MESSAGES_DEBUG to 'Spice' impacts
> > spice_debug() and
> >   * g_debug() but not other_debug() */
> >  static void test_spice_g_messages_debug(void)
> > @@ -325,9 +32,9 @@ static void test_spice_g_messages_debug(void)
> >          g_debug("g_debug");
> >          g_info("g_info");
> >          g_message("g_message");
> > -        other_debug("other_debug");
> > -        other_info("other_info");
> > -        other_message("other_message");
> > +        g_log("Other", G_LOG_LEVEL_DEBUG, "other_debug");
> > +        g_log("Other", G_LOG_LEVEL_INFO, "other_info");
> > +        g_log("Other", G_LOG_LEVEL_MESSAGE, "other_message");
> >  
> >          return;
> >      }
> > @@ -350,9 +57,9 @@ static void test_spice_g_messages_debug_all(void)
> >          g_debug("g_debug");
> >          g_info("g_info");
> >          g_message("g_message");
> > -        other_debug("other_debug");
> > -        other_info("other_info");
> > -        other_message("other_message");
> > +        g_log("Other", G_LOG_LEVEL_DEBUG, "other_debug");
> > +        g_log("Other", G_LOG_LEVEL_INFO, "other_info");
> > +        g_log("Other", G_LOG_LEVEL_MESSAGE, "other_message");
> >  
> >          return;
> >      }
> > @@ -387,19 +94,8 @@ int main(int argc, char **argv)
> >      g_log_set_always_fatal(fatal_mask & G_LOG_LEVEL_MASK);
> >  
> >  #if GLIB_CHECK_VERSION(2,38,0)
> > -    g_test_add_func("/spice-common/spice-abort-level",
> > test_spice_abort_level);
> > -    g_test_add_func("/spice-common/spice-abort-level-gwarning",
> > test_spice_abort_level_g_warning);
> > -    g_test_add_func("/spice-common/spice-debug-level",
> > test_spice_debug_level);
> > -    g_test_add_func("/spice-common/spice-debug-level-warning",
> > test_spice_debug_level_warning);
> >      g_test_add_func("/spice-common/spice-g-messages-debug",
> > test_spice_g_messages_debug);
> >      g_test_add_func("/spice-common/spice-g-messages-debug-all",
> > test_spice_g_messages_debug_all);
> > -    g_test_add_func("/spice-common/spice-log-levels",
> > test_log_levels);
> > -    g_test_add_func("/spice-common/spice-fatal-critical",
> > test_spice_fatal_critical);
> > -    g_test_add_func("/spice-common/spice-non-fatal-gcritical",
> > test_spice_non_fatal_g_critical);
> > -    g_test_add_func("/spice-common/spice-fatal-gcritical",
> > test_spice_fatal_g_critical);
> > -    g_test_add_func("/spice-common/spice-fatal-return-if-fail",
> > test_spice_fatal_return_if_fail);
> > -    g_test_add_func("/spice-common/spice-non-fatal-greturn-if-fail",
> > test_spice_non_fatal_g_return_if_fail);
> > -    g_test_add_func("/spice-common/spice-fatal-warning",
> > test_spice_fatal_warning);
> >  #endif /* GLIB_CHECK_VERSION(2,38,0) */
> >  
> >      return g_test_run();
> 


More information about the Spice-devel mailing list