[PATCH 1/5] log: Refactor evaluation of log level

Aleksander Morgado aleksander at aleksander.es
Wed Jun 21 11:34:59 UTC 2017


On 21/06/17 12:54, Torsten Hilbrich wrote:
> Starting with adding a typed enum type for the log level named
> MMLogLevel.
> 
> Suggested-By: Aleksander Morgado <aleksander at aleksander.es>
> 
> The level was checked twice in _mm_log. Once at the beginning and
> again when turning the level into both the syslog priority and some
> descriptive text which was added to the log level. Removing the check
> at the second location as it was redundant.
> 
> Also adding a helper function to turn the MMLogLevel into the syslog
> equivalent. This will become handy when adding support for systemd
> journal.

Pushed to git master, thanks.

> ---
>  src/mm-log.c | 54 ++++++++++++++++++++++++++++++++++++------------------
>  src/mm-log.h | 22 +++++++++++-----------
>  2 files changed, 47 insertions(+), 29 deletions(-)
> 
> diff --git a/src/mm-log.c b/src/mm-log.c
> index cb2b5cf..4f882f3 100644
> --- a/src/mm-log.c
> +++ b/src/mm-log.c
> @@ -44,7 +44,7 @@ enum {
>  };
>  
>  static gboolean ts_flags = TS_FLAG_NONE;
> -static guint32 log_level = LOGL_INFO | LOGL_WARN | LOGL_ERR;
> +static guint32 log_level = MM_LOG_LEVEL_INFO | MM_LOG_LEVEL_WARN | MM_LOG_LEVEL_ERR;
>  static GTimeVal rel_start = { 0, 0 };
>  static int logfd = -1;
>  
> @@ -54,26 +54,42 @@ typedef struct {
>  } LogDesc;
>  
>  static const LogDesc level_descs[] = {
> -    { LOGL_ERR, "ERR" },
> -    { LOGL_WARN | LOGL_ERR, "WARN" },
> -    { LOGL_INFO | LOGL_WARN | LOGL_ERR, "INFO" },
> -    { LOGL_DEBUG | LOGL_INFO | LOGL_WARN | LOGL_ERR, "DEBUG" },
> +    { MM_LOG_LEVEL_ERR, "ERR" },
> +    { MM_LOG_LEVEL_WARN | MM_LOG_LEVEL_ERR, "WARN" },
> +    { MM_LOG_LEVEL_INFO | MM_LOG_LEVEL_WARN | MM_LOG_LEVEL_ERR, "INFO" },
> +    { MM_LOG_LEVEL_DEBUG | MM_LOG_LEVEL_INFO | MM_LOG_LEVEL_WARN | MM_LOG_LEVEL_ERR, "DEBUG" },
>      { 0, NULL }
>  };
>  
>  static GString *msgbuf = NULL;
>  static volatile gsize msgbuf_once = 0;
>  
> +static int
> +mm_to_syslog_priority (MMLogLevel level)
> +{
> +    switch (level) {
> +    case MM_LOG_LEVEL_DEBUG:
> +        return LOG_DEBUG;
> +    case MM_LOG_LEVEL_WARN:
> +        return LOG_WARNING;
> +    case MM_LOG_LEVEL_INFO:
> +        return LOG_INFO;
> +    case MM_LOG_LEVEL_ERR:
> +        return LOG_ERR;
> +    }
> +    g_assert_not_reached();
> +    return 0;
> +}
> +
>  void
>  _mm_log (const char *loc,
>           const char *func,
> -         guint32 level,
> +         MMLogLevel level,
>           const char *fmt,
>           ...)
>  {
>      va_list args;
>      GTimeVal tv;
> -    int syslog_priority = LOG_INFO;
>      ssize_t ign;
>  
>      if (!(log_level & level))
> @@ -85,18 +101,20 @@ _mm_log (const char *loc,
>      } else
>          g_string_truncate (msgbuf, 0);
>  
> -    if ((log_level & LOGL_DEBUG) && (level == LOGL_DEBUG))
> +    switch (level) {
> +    case MM_LOG_LEVEL_DEBUG:
>          g_string_append (msgbuf, "<debug> ");
> -    else if ((log_level & LOGL_INFO) && (level == LOGL_INFO))
> +        break;
> +    case MM_LOG_LEVEL_INFO:
>          g_string_append (msgbuf, "<info>  ");
> -    else if ((log_level & LOGL_WARN) && (level == LOGL_WARN)) {
> +        break;
> +    case MM_LOG_LEVEL_WARN:
>          g_string_append (msgbuf, "<warn>  ");
> -        syslog_priority = LOG_WARNING;
> -    } else if ((log_level & LOGL_ERR) && (level == LOGL_ERR)) {
> +        break;
> +    case MM_LOG_LEVEL_ERR:
>          g_string_append (msgbuf, "<error> ");
> -        syslog_priority = LOG_ERR;
> -    } else
> -        return;
> +        break;
> +    }
>  
>      if (ts_flags == TS_FLAG_WALL) {
>          g_get_current_time (&tv);
> @@ -127,7 +145,7 @@ _mm_log (const char *loc,
>      g_string_append_c (msgbuf, '\n');
>  
>      if (logfd < 0)
> -        syslog (syslog_priority, "%s", msgbuf->str);
> +        syslog (mm_to_syslog_priority (level), "%s", msgbuf->str);
>      else {
>          ign = write (logfd, msgbuf->str, msgbuf->len);
>          if (ign) {} /* whatever; really shut up about unused result */
> @@ -194,11 +212,11 @@ mm_log_set_level (const char *level, GError **error)
>                       "Unknown log level '%s'", level);
>  
>  #if defined WITH_QMI
> -    qmi_utils_set_traces_enabled (log_level & LOGL_DEBUG ? TRUE : FALSE);
> +    qmi_utils_set_traces_enabled (log_level & MM_LOG_LEVEL_DEBUG ? TRUE : FALSE);
>  #endif
>  
>  #if defined WITH_MBIM
> -    mbim_utils_set_traces_enabled (log_level & LOGL_DEBUG ? TRUE : FALSE);
> +    mbim_utils_set_traces_enabled (log_level & MM_LOG_LEVEL_DEBUG ? TRUE : FALSE);
>  #endif
>  
>      return found;
> diff --git a/src/mm-log.h b/src/mm-log.h
> index 292d68a..6c34098 100644
> --- a/src/mm-log.h
> +++ b/src/mm-log.h
> @@ -19,31 +19,31 @@
>  #include <glib.h>
>  
>  /* Log levels */
> -enum {
> -    LOGL_ERR   = 0x00000001,
> -    LOGL_WARN  = 0x00000002,
> -    LOGL_INFO  = 0x00000004,
> -    LOGL_DEBUG = 0x00000008
> -};
> +typedef enum {
> +    MM_LOG_LEVEL_ERR   = 0x00000001,
> +    MM_LOG_LEVEL_WARN  = 0x00000002,
> +    MM_LOG_LEVEL_INFO  = 0x00000004,
> +    MM_LOG_LEVEL_DEBUG = 0x00000008
> +} MMLogLevel;
>  
>  #define mm_err(...) \
> -    _mm_log (G_STRLOC, G_STRFUNC, LOGL_ERR, ## __VA_ARGS__ )
> +    _mm_log (G_STRLOC, G_STRFUNC, MM_LOG_LEVEL_ERR, ## __VA_ARGS__ )
>  
>  #define mm_warn(...) \
> -    _mm_log (G_STRLOC, G_STRFUNC, LOGL_WARN, ## __VA_ARGS__ )
> +    _mm_log (G_STRLOC, G_STRFUNC, MM_LOG_LEVEL_WARN, ## __VA_ARGS__ )
>  
>  #define mm_info(...) \
> -    _mm_log (G_STRLOC, G_STRFUNC, LOGL_INFO, ## __VA_ARGS__ )
> +    _mm_log (G_STRLOC, G_STRFUNC, MM_LOG_LEVEL_INFO, ## __VA_ARGS__ )
>  
>  #define mm_dbg(...) \
> -    _mm_log (G_STRLOC, G_STRFUNC, LOGL_DEBUG, ## __VA_ARGS__ )
> +    _mm_log (G_STRLOC, G_STRFUNC, MM_LOG_LEVEL_DEBUG, ## __VA_ARGS__ )
>  
>  #define mm_log(level, ...) \
>      _mm_log (G_STRLOC, G_STRFUNC, level, ## __VA_ARGS__ )
>  
>  void _mm_log (const char *loc,
>                const char *func,
> -              guint32 level,
> +              MMLogLevel level,
>                const char *fmt,
>                ...)  __attribute__((__format__ (__printf__, 4, 5)));
>  
> 


-- 
Aleksander
https://aleksander.es


More information about the ModemManager-devel mailing list