From 553b04148a0d145d011515f26019bf6e05ba605f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 19 May 2016 18:57:18 +0200 Subject: [PATCH 1/4] logging: drop explicit initialization of nm-logging Instead of calling _ensure_initialized() at various places to ensure that we setup logging before any logging commands are executed, initialize the logging fields in the global variable. This removes code from nm_logging_enabled(), which we want to become a static inline function. --- src/nm-logging.c | 36 ++++++++---------------------------- 1 file changed, 8 insertions(+), 28 deletions(-) diff --git a/src/nm-logging.c b/src/nm-logging.c index 935b34a8e..c00b56de4 100644 --- a/src/nm-logging.c +++ b/src/nm-logging.c @@ -101,7 +101,6 @@ typedef struct { static struct { NMLogLevel log_level; NMLogDomain logging[_LOGL_N_REAL]; - gboolean logging_set_up; LogFormatFlags log_format_flags; enum { LOG_BACKEND_GLIB, @@ -116,7 +115,13 @@ static struct { * but that feature doesn't seem well supported. */ const LogDesc domain_desc[_DOMAIN_DESC_LEN]; } global = { + /* nm_logging_setup ("INFO", LOGD_DEFAULT_STRING, NULL, NULL); */ .log_level = LOGL_INFO, + .logging = { + [LOGL_INFO] = LOGD_DEFAULT, + [LOGL_WARN] = LOGD_DEFAULT, + [LOGL_ERR] = LOGD_DEFAULT, + }, .log_backend = LOG_BACKEND_GLIB, .log_format_flags = _LOG_FORMAT_FLAG_DEFAULT, .level_desc = { @@ -185,19 +190,6 @@ static char *_domains_to_string (gboolean include_level_override); /************************************************************************/ -static void -_ensure_initialized (void) -{ - if (G_UNLIKELY (!global.logging_set_up)) { - int errsv = errno; - - nm_logging_setup ("INFO", LOGD_DEFAULT_STRING, NULL, NULL); - - /* must ensure that errno is not modified. */ - errno = errsv; - } -} - static gboolean match_log_level (const char *level, NMLogLevel *out_level, @@ -235,13 +227,8 @@ nm_logging_setup (const char *level, g_return_val_if_fail (!error || !*error, FALSE); /* domains */ - if (!domains || !*domains) { - domains = global.logging_set_up - ? (domains_free = _domains_to_string (FALSE)) - : LOGD_DEFAULT_STRING; - } - - global.logging_set_up = TRUE; + if (!domains || !*domains) + domains = (domains_free = _domains_to_string (FALSE)); for (i = 0; i < G_N_ELEMENTS (new_logging); i++) new_logging[i] = 0; @@ -384,8 +371,6 @@ nm_logging_all_levels_to_string (void) const char * nm_logging_domains_to_string (void) { - _ensure_initialized (); - if (G_UNLIKELY (!global.logging_domains_to_string)) global.logging_domains_to_string = _domains_to_string (TRUE); @@ -465,9 +450,6 @@ nm_logging_enabled (NMLogLevel level, NMLogDomain domain) if ((guint) level >= G_N_ELEMENTS (global.logging)) g_return_val_if_reached (FALSE); - /* This function is guaranteed not to modify errno. */ - _ensure_initialized (); - return !!(global.logging[level] & domain); } @@ -518,8 +500,6 @@ _nm_log_impl (const char *file, if ((guint) level >= G_N_ELEMENTS (global.logging)) g_return_if_reached (); - _ensure_initialized (); - if (!(global.logging[level] & domain)) return; From 1a070f6a44a0f1a39c174425be96dd8834bea25e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 19 May 2016 18:58:42 +0200 Subject: [PATCH 2/4] logging: remove assertion in nm_logging_enabled() from production builds We really expect this assertion not to be violated. As we want for nm_logging_enabled() to become smaller and inline, remove the runtime assertion from regular builds. Live fast and dangerous. --- src/nm-logging.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/nm-logging.c b/src/nm-logging.c index c00b56de4..30932440c 100644 --- a/src/nm-logging.c +++ b/src/nm-logging.c @@ -447,8 +447,7 @@ nm_logging_all_domains_to_string (void) gboolean nm_logging_enabled (NMLogLevel level, NMLogDomain domain) { - if ((guint) level >= G_N_ELEMENTS (global.logging)) - g_return_val_if_reached (FALSE); + nm_assert (((guint) level) < G_N_ELEMENTS (global.logging)); return !!(global.logging[level] & domain); } From a9ef2f9c5092a7cd6dbb52aa2b0c18c3cca7627d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 19 May 2016 19:01:58 +0200 Subject: [PATCH 3/4] logging: move static variable with logging state to file-scope --- src/nm-logging.c | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/src/nm-logging.c b/src/nm-logging.c index 30932440c..ded12ed03 100644 --- a/src/nm-logging.c +++ b/src/nm-logging.c @@ -98,9 +98,15 @@ typedef struct { LogFormatFlags log_format_level; } LogLevelDesc; +static NMLogDomain _nm_logging_enabled_state[_LOGL_N_REAL] = { + /* nm_logging_setup ("INFO", LOGD_DEFAULT_STRING, NULL, NULL); */ + [LOGL_INFO] = LOGD_DEFAULT, + [LOGL_WARN] = LOGD_DEFAULT, + [LOGL_ERR] = LOGD_DEFAULT, +}; + static struct { NMLogLevel log_level; - NMLogDomain logging[_LOGL_N_REAL]; LogFormatFlags log_format_flags; enum { LOG_BACKEND_GLIB, @@ -117,11 +123,6 @@ static struct { } global = { /* nm_logging_setup ("INFO", LOGD_DEFAULT_STRING, NULL, NULL); */ .log_level = LOGL_INFO, - .logging = { - [LOGL_INFO] = LOGD_DEFAULT, - [LOGL_WARN] = LOGD_DEFAULT, - [LOGL_ERR] = LOGD_DEFAULT, - }, .log_backend = LOG_BACKEND_GLIB, .log_format_flags = _LOG_FORMAT_FLAG_DEFAULT, .level_desc = { @@ -216,7 +217,7 @@ nm_logging_setup (const char *level, GError **error) { GString *unrecognized = NULL; - NMLogDomain new_logging[G_N_ELEMENTS (global.logging)]; + NMLogDomain new_logging[G_N_ELEMENTS (_nm_logging_enabled_state)]; NMLogLevel new_log_level = global.log_level; char **tmp, **iter; int i; @@ -240,7 +241,7 @@ nm_logging_setup (const char *level, if (new_log_level == _LOGL_KEEP) { new_log_level = global.log_level; for (i = 0; i < G_N_ELEMENTS (new_logging); i++) - new_logging[i] = global.logging[i]; + new_logging[i] = _nm_logging_enabled_state[i]; } } @@ -308,7 +309,7 @@ nm_logging_setup (const char *level, if (domain_log_level == _LOGL_KEEP) { for (i = 0; i < G_N_ELEMENTS (new_logging); i++) - new_logging[i] = (new_logging[i] & ~bits) | (global.logging[i] & bits); + new_logging[i] = (new_logging[i] & ~bits) | (_nm_logging_enabled_state[i] & bits); } else { for (i = 0; i < G_N_ELEMENTS (new_logging); i++) { if (i < domain_log_level) @@ -326,7 +327,7 @@ nm_logging_setup (const char *level, global.log_level = new_log_level; for (i = 0; i < G_N_ELEMENTS (new_logging); i++) - global.logging[i] = new_logging[i]; + _nm_logging_enabled_state[i] = new_logging[i]; if ( had_platform_debug && _nm_logging_clear_platform_logging_cache @@ -391,7 +392,7 @@ _domains_to_string (gboolean include_level_override) str = g_string_sized_new (75); for (diter = &global.domain_desc[0]; diter->name; diter++) { /* If it's set for any lower level, it will also be set for LOGL_ERR */ - if (!(diter->num & global.logging[LOGL_ERR])) + if (!(diter->num & _nm_logging_enabled_state[LOGL_ERR])) continue; if (str->len) @@ -403,15 +404,15 @@ _domains_to_string (gboolean include_level_override) /* Check if it's logging at a lower level than the default. */ for (i = 0; i < global.log_level; i++) { - if (diter->num & global.logging[i]) { + if (diter->num & _nm_logging_enabled_state[i]) { g_string_append_printf (str, ":%s", global.level_desc[i].name); break; } } /* Check if it's logging at a higher level than the default. */ - if (!(diter->num & global.logging[global.log_level])) { - for (i = global.log_level + 1; i < G_N_ELEMENTS (global.logging); i++) { - if (diter->num & global.logging[i]) { + if (!(diter->num & _nm_logging_enabled_state[global.log_level])) { + for (i = global.log_level + 1; i < G_N_ELEMENTS (_nm_logging_enabled_state); i++) { + if (diter->num & _nm_logging_enabled_state[i]) { g_string_append_printf (str, ":%s", global.level_desc[i].name); break; } @@ -447,9 +448,9 @@ nm_logging_all_domains_to_string (void) gboolean nm_logging_enabled (NMLogLevel level, NMLogDomain domain) { - nm_assert (((guint) level) < G_N_ELEMENTS (global.logging)); + nm_assert (((guint) level) < G_N_ELEMENTS (_nm_logging_enabled_state)); - return !!(global.logging[level] & domain); + return !!(_nm_logging_enabled_state[level] & domain); } #if SYSTEMD_JOURNAL @@ -496,10 +497,10 @@ _nm_log_impl (const char *file, char s_buf_location[1024]; GTimeVal tv; - if ((guint) level >= G_N_ELEMENTS (global.logging)) + if ((guint) level >= G_N_ELEMENTS (_nm_logging_enabled_state)) g_return_if_reached (); - if (!(global.logging[level] & domain)) + if (!(_nm_logging_enabled_state[level] & domain)) return; /* Make sure that %m maps to the specified error */ @@ -588,7 +589,7 @@ _nm_log_impl (const char *file, const char *s_domain_1 = NULL; GString *s_domain_all = NULL; NMLogDomain dom_all = domain; - NMLogDomain dom = dom_all & global.logging[level]; + NMLogDomain dom = dom_all & _nm_logging_enabled_state[level]; for (diter = &global.domain_desc[0]; diter->name; diter++) { if (!NM_FLAGS_HAS (dom_all, diter->num)) From 4ed1784ce4d89b905ecfd75460f8c73893401442 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 19 May 2016 19:05:43 +0200 Subject: [PATCH 4/4] logging: make nm_logging_enabled() inline function Basically every logging statement is wrapped by a nm_logging_enabled() to evaluate the function call of the logging lazy. Make the function a candidate for inlining, it safes some space. On a default build it goes for me from 2580584 to 2560104 bytes (20k). $ ./autogen.sh && make && strip ./src/NetworkManager --- src/nm-logging.c | 10 +--------- src/nm-logging.h | 9 ++++++++- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/nm-logging.c b/src/nm-logging.c index ded12ed03..a2581add2 100644 --- a/src/nm-logging.c +++ b/src/nm-logging.c @@ -98,7 +98,7 @@ typedef struct { LogFormatFlags log_format_level; } LogLevelDesc; -static NMLogDomain _nm_logging_enabled_state[_LOGL_N_REAL] = { +NMLogDomain _nm_logging_enabled_state[_LOGL_N_REAL] = { /* nm_logging_setup ("INFO", LOGD_DEFAULT_STRING, NULL, NULL); */ [LOGL_INFO] = LOGD_DEFAULT, [LOGL_WARN] = LOGD_DEFAULT, @@ -445,14 +445,6 @@ nm_logging_all_domains_to_string (void) return str->str; } -gboolean -nm_logging_enabled (NMLogLevel level, NMLogDomain domain) -{ - nm_assert (((guint) level) < G_N_ELEMENTS (_nm_logging_enabled_state)); - - return !!(_nm_logging_enabled_state[level] & domain); -} - #if SYSTEMD_JOURNAL __attribute__((__format__ (__printf__, 4, 5))) static void diff --git a/src/nm-logging.h b/src/nm-logging.h index 27f89f5d5..c13467050 100644 --- a/src/nm-logging.h +++ b/src/nm-logging.h @@ -158,7 +158,14 @@ void _nm_log_impl (const char *file, const char *nm_logging_level_to_string (void); const char *nm_logging_domains_to_string (void); -gboolean nm_logging_enabled (NMLogLevel level, NMLogDomain domain); + +extern NMLogDomain _nm_logging_enabled_state[_LOGL_N_REAL]; +static inline gboolean +nm_logging_enabled (NMLogLevel level, NMLogDomain domain) +{ + nm_assert (((guint) level) < G_N_ELEMENTS (_nm_logging_enabled_state)); + return !!(_nm_logging_enabled_state[level] & domain); +} const char *nm_logging_all_levels_to_string (void); const char *nm_logging_all_domains_to_string (void);