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

Torsten Hilbrich torsten.hilbrich at secunet.com
Wed Jun 21 10:54:35 UTC 2017


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.
---
 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)));
 
-- 
2.7.4



More information about the ModemManager-devel mailing list