On Wed, Jun 07, 2017 at 11:22:13AM +0100, Frediano Ziglio wrote:
> As we moved toward GLib logging instead of having to convert
> every time the log level from the old system to GLib use
> directly GLib constants.
>
> Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> ---
> common/log.c | 35 +++++++++++++++++++++--------------
> common/log.h | 26 +++++++++-----------------
> 2 files changed, 30 insertions(+), 31 deletions(-)
>
> diff --git a/common/log.c b/common/log.c
> index 7437d3e..14b1c1c 100644
> --- a/common/log.c
> +++ b/common/log.c
> @@ -31,16 +31,24 @@
> #include "backtrace.h"
>
> static int glib_debug_level = INT_MAX;
> -static int abort_level = -1;
> +static int abort_levels = -1;
I think I'd go with abort_mask rather than abort_levels.
It looks like you can initialize it to 0, and save a != 0 test in
spice_logv.
>
> -#ifndef SPICE_ABORT_LEVEL_DEFAULT
> +#ifndef SPICE_ABORT_LEVELS_DEFAULT
> #ifdef SPICE_DISABLE_ABORT
> -#define SPICE_ABORT_LEVEL_DEFAULT -1
> +#define SPICE_ABORT_LEVELS_DEFAULT 0
> #else
> -#define SPICE_ABORT_LEVEL_DEFAULT SPICE_LOG_LEVEL_CRITICAL
> +#define SPICE_ABORT_LEVELS_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[] = {
> @@ -95,23 +103,23 @@ static void spice_log_set_debug_level(void)
>
> static void spice_log_set_abort_level(void)
> {
> - if (abort_level == -1) {
> + if (abort_levels == -1) {
> 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");
> - abort_level = atoi(abort_str);
> - glib_abort_level = spice_log_level_to_glib(abort_level);
> + 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_levels = fatal_mask;
> g_log_set_fatal_mask(SPICE_LOG_DOMAIN, fatal_mask);
> } else {
> - abort_level = SPICE_ABORT_LEVEL_DEFAULT;
> + abort_levels = SPICE_ABORT_LEVELS_DEFAULT;
> }
> }
> }
> @@ -146,16 +154,15 @@ SPICE_CONSTRUCTOR_FUNC(spice_log_init)
> }
>
> static void spice_logv(const char *log_domain,
> - SpiceLogLevel log_level,
> + GLogLevelFlags log_level,
> const char *strloc,
> const char *function,
> const char *format,
> va_list args)
> {
> GString *log_msg;
> - GLogLevelFlags glib_level = spice_log_level_to_glib(log_level);
>
> - if ((glib_level & G_LOG_LEVEL_MASK) > glib_debug_level) {
> + if ((log_level & G_LOG_LEVEL_MASK) > glib_debug_level) {
> return; // do not print anything
> }
>
> @@ -166,17 +173,17 @@ static void spice_logv(const char *log_domain,
> if (format) {
> g_string_append_vprintf(log_msg, format, args);
> }
> - g_log(log_domain, glib_level, "%s", log_msg->str);
> + g_log(log_domain, log_level, "%s", log_msg->str);
> g_string_free(log_msg, TRUE);
>
> - if (abort_level != -1 && abort_level >= (int) log_level) {
> + if (abort_levels != -1 && (abort_levels & log_level) != 0) {
> spice_backtrace();
> abort();
> }
I'm wondering if this codepath can be hit at all, I'd expect
g_log_set_fatal_mask to cause g_log to abort before this is reached.
(not a problem with your patch ;)
Acked-by: Christophe Fergeau <cfergeau@xxxxxxxxxx>
Christophe
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel