From edeaccc9eb6aab762ef44fa61e1e7f0e2f9a347d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 5 Dec 2014 00:54:30 +0100 Subject: [PATCH 1/7] logging: add error argument to _nm_log() to support "%m" format specifier A gnu extension to printf adds the format specifier "%m" to print @errno. To preserve the error number until the point where the logging statement is constructed, pass it as an additional argument to _nm_log(). This is not (yet) used from NM internal code. But systemd is adding similar functionality to its logging functions. Add the same also to nm-logging, to support systemd's usage of "%m". (cherry picked from commit 5162426d416a4d309602d695c5966d109fc7bcc0) --- src/dhcp-manager/systemd-dhcp/nm-sd-adapt.h | 2 +- src/nm-logging.c | 5 +++++ src/nm-logging.h | 5 +++-- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/dhcp-manager/systemd-dhcp/nm-sd-adapt.h b/src/dhcp-manager/systemd-dhcp/nm-sd-adapt.h index ce1716b175..a95f5bfb42 100644 --- a/src/dhcp-manager/systemd-dhcp/nm-sd-adapt.h +++ b/src/dhcp-manager/systemd-dhcp/nm-sd-adapt.h @@ -57,7 +57,7 @@ G_STMT_START { \ if (nm_logging_enabled (_l, LOGD_DHCP)) { \ const char *_location = strrchr (file "", '/'); \ \ - _nm_log (_location ? _location + 1 : file, line, func, _l, LOGD_DHCP, format, ## __VA_ARGS__); \ + _nm_log (_location ? _location + 1 : file, line, func, _l, LOGD_DHCP, 0, format, ## __VA_ARGS__); \ } \ } G_STMT_END diff --git a/src/nm-logging.c b/src/nm-logging.c index f1291fa73b..2778e84b89 100644 --- a/src/nm-logging.c +++ b/src/nm-logging.c @@ -362,6 +362,7 @@ _nm_log (const char *file, const char *func, NMLogLevel level, NMLogDomain domain, + int error, const char *fmt, ...) { @@ -379,6 +380,10 @@ _nm_log (const char *file, if (!(logging[level] & domain)) return; + /* Make sure that %m maps to the specified error */ + if (error != 0) + errno = error; + va_start (args, fmt); msg = g_strdup_vprintf (fmt, args); va_end (args); diff --git a/src/nm-logging.h b/src/nm-logging.h index e0aa71690a..4ae9c02687 100644 --- a/src/nm-logging.h +++ b/src/nm-logging.h @@ -105,7 +105,7 @@ typedef enum { /*< skip >*/ #define nm_log(level, domain, ...) \ G_STMT_START { \ if (nm_logging_enabled ((level), (domain))) { \ - _nm_log (__FILE__, __LINE__, G_STRFUNC, (level), (domain), __VA_ARGS__); \ + _nm_log (__FILE__, __LINE__, G_STRFUNC, (level), (domain), 0, __VA_ARGS__); \ } \ } G_STMT_END @@ -141,8 +141,9 @@ void _nm_log (const char *file, const char *func, NMLogLevel level, NMLogDomain domain, + int error, const char *fmt, - ...) __attribute__((__format__ (__printf__, 6, 7))); + ...) __attribute__((__format__ (__printf__, 7, 8))); const char *nm_logging_level_to_string (void); const char *nm_logging_domains_to_string (void); From b3cae064d16941e3381824000856dfde9e7aa201 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 22 Apr 2015 10:20:23 +0200 Subject: [PATCH 2/7] ibft/logging: don't localize logging stagements We don't localize any other nm-logging messages either. (cherry picked from commit b04c99da0bbde6e8f27f718bea46b9b2787c3236) --- src/settings/plugins/ibft/plugin.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/settings/plugins/ibft/plugin.c b/src/settings/plugins/ibft/plugin.c index 303971e05b..632c4df14b 100644 --- a/src/settings/plugins/ibft/plugin.c +++ b/src/settings/plugins/ibft/plugin.c @@ -63,7 +63,7 @@ read_connections (SCPluginIbft *self) NMIbftConnection *connection; if (!read_ibft_blocks ("/sbin/iscsiadm", &blocks, &error)) { - nm_log_dbg (LOGD_SETTINGS, _("ibft: failed to read iscsiadm records: %s"), error->message); + nm_log_dbg (LOGD_SETTINGS, "ibft: failed to read iscsiadm records: %s", error->message); g_error_free (error); return; } @@ -71,13 +71,13 @@ read_connections (SCPluginIbft *self) for (iter = blocks; iter; iter = iter->next) { connection = nm_ibft_connection_new (iter->data, &error); if (connection) { - nm_log_info (LOGD_SETTINGS, _("ibft: read connection '%s'"), + nm_log_info (LOGD_SETTINGS, "ibft: read connection '%s'", nm_connection_get_id (NM_CONNECTION (connection))); g_hash_table_insert (priv->connections, g_strdup (nm_connection_get_uuid (NM_CONNECTION (connection))), connection); } else { - nm_log_warn (LOGD_SETTINGS, _("ibft: failed to read iscsiadm record: %s"), error->message); + nm_log_warn (LOGD_SETTINGS, "ibft: failed to read iscsiadm record: %s", error->message); g_clear_error (&error); } } From 2f53e0dfbcc2f1898d42c24cafaba920d7ec8d37 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 22 Apr 2015 10:19:35 +0200 Subject: [PATCH 3/7] logging: always pass a static format string to logging functions (cherry picked from commit bdec5e2e5375faa9752738d070a287b72553b029) --- src/nm-manager.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/nm-manager.c b/src/nm-manager.c index 25f9533af4..0fa88d1d8a 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -3630,7 +3630,7 @@ do_sleep_wake (NMManager *self, gboolean sleeping_changed) waking_from_suspend = sleeping_changed && !priv->sleeping; if (manager_sleeping (self)) { - nm_log_info (LOGD_SUSPEND, suspending ? "sleeping..." : "disabling..."); + nm_log_info (LOGD_SUSPEND, "%s...", suspending ? "sleeping" : "disabling"); /* FIXME: are there still hardware devices that need to be disabled around * suspend/resume? @@ -3648,7 +3648,7 @@ do_sleep_wake (NMManager *self, gboolean sleeping_changed) nm_device_set_unmanaged (device, NM_UNMANAGED_INTERNAL, TRUE, NM_DEVICE_STATE_REASON_SLEEPING); } } else { - nm_log_info (LOGD_SUSPEND, waking_from_suspend ? "waking up..." : "re-enabling..."); + nm_log_info (LOGD_SUSPEND, "%s...", waking_from_suspend ? "waking up" : "re-enabling"); if (waking_from_suspend) { /* Belatedly take down Wake-on-LAN devices; ideally we wouldn't have to do this From 97f9b03119c86edfa79ed2a3652b2852592dbb4f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 22 Apr 2015 10:13:06 +0200 Subject: [PATCH 4/7] logging: ensure that the first argument of the logging statement is a C string We don't want to pass unknown format strings to the logging macro. Catch that by concatenating "" with the format string. (cherry picked from commit 2458ddf5e95ed74613c32780622e265eebb7e712) --- src/nm-logging.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nm-logging.h b/src/nm-logging.h index 4ae9c02687..bbecd6ef9b 100644 --- a/src/nm-logging.h +++ b/src/nm-logging.h @@ -105,7 +105,7 @@ typedef enum { /*< skip >*/ #define nm_log(level, domain, ...) \ G_STMT_START { \ if (nm_logging_enabled ((level), (domain))) { \ - _nm_log (__FILE__, __LINE__, G_STRFUNC, (level), (domain), 0, __VA_ARGS__); \ + _nm_log (__FILE__, __LINE__, G_STRFUNC, (level), (domain), 0, ""__VA_ARGS__); \ } \ } G_STMT_END From 96c3f7b39f2ce602aa2cf0b641b3cfc0da36d5cf Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 22 Apr 2015 11:06:49 +0200 Subject: [PATCH 5/7] logging/trivial: rename _nm_log() to _nm_log_impl() The actual logging implementation is not supposed to be called directly, because there are macros that capture the call site information __FILE__, __LINE__, and G_STRFUNC. Rename the function to make clear that this is the actual implementation. (cherry picked from commit 211d241ab081361341f4eeb05d7175993bde514c) Conflicts: src/dhcp-manager/systemd-dhcp/nm-sd-adapt.h --- src/dhcp-manager/systemd-dhcp/nm-sd-adapt.h | 2 +- src/nm-logging.c | 16 ++++++++-------- src/nm-logging.h | 18 +++++++++--------- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/dhcp-manager/systemd-dhcp/nm-sd-adapt.h b/src/dhcp-manager/systemd-dhcp/nm-sd-adapt.h index a95f5bfb42..c316fb4692 100644 --- a/src/dhcp-manager/systemd-dhcp/nm-sd-adapt.h +++ b/src/dhcp-manager/systemd-dhcp/nm-sd-adapt.h @@ -57,7 +57,7 @@ G_STMT_START { \ if (nm_logging_enabled (_l, LOGD_DHCP)) { \ const char *_location = strrchr (file "", '/'); \ \ - _nm_log (_location ? _location + 1 : file, line, func, _l, LOGD_DHCP, 0, format, ## __VA_ARGS__); \ + _nm_log_impl (_location ? _location + 1 : file, line, func, _l, LOGD_DHCP, 0, format, ## __VA_ARGS__); \ } \ } G_STMT_END diff --git a/src/nm-logging.c b/src/nm-logging.c index 2778e84b89..fa770344ed 100644 --- a/src/nm-logging.c +++ b/src/nm-logging.c @@ -357,14 +357,14 @@ nm_logging_enabled (NMLogLevel level, NMLogDomain domain) } void -_nm_log (const char *file, - guint line, - const char *func, - NMLogLevel level, - NMLogDomain domain, - int error, - const char *fmt, - ...) +_nm_log_impl (const char *file, + guint line, + const char *func, + NMLogLevel level, + NMLogDomain domain, + int error, + const char *fmt, + ...) { va_list args; char *msg; diff --git a/src/nm-logging.h b/src/nm-logging.h index bbecd6ef9b..ddfa258cbd 100644 --- a/src/nm-logging.h +++ b/src/nm-logging.h @@ -105,7 +105,7 @@ typedef enum { /*< skip >*/ #define nm_log(level, domain, ...) \ G_STMT_START { \ if (nm_logging_enabled ((level), (domain))) { \ - _nm_log (__FILE__, __LINE__, G_STRFUNC, (level), (domain), 0, ""__VA_ARGS__); \ + _nm_log_impl (__FILE__, __LINE__, G_STRFUNC, (level), (domain), 0, ""__VA_ARGS__); \ } \ } G_STMT_END @@ -136,14 +136,14 @@ typedef enum { /*< skip >*/ nm_log_ptr ((level), (domain), (self), __VA_ARGS__) -void _nm_log (const char *file, - guint line, - const char *func, - NMLogLevel level, - NMLogDomain domain, - int error, - const char *fmt, - ...) __attribute__((__format__ (__printf__, 7, 8))); +void _nm_log_impl (const char *file, + guint line, + const char *func, + NMLogLevel level, + NMLogDomain domain, + int error, + const char *fmt, + ...) __attribute__((__format__ (__printf__, 7, 8))); const char *nm_logging_level_to_string (void); const char *nm_logging_domains_to_string (void); From 6d8d5674441d0feb52e3697360dc88e48568425d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 22 Apr 2015 11:11:44 +0200 Subject: [PATCH 6/7] logging: add logging macro _nm_log() that logs unconditionally (cherry picked from commit fb9f8c69f7c665b9e1ed4358a5bc41548263b0fa) --- src/nm-logging.h | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/nm-logging.h b/src/nm-logging.h index ddfa258cbd..00d429b095 100644 --- a/src/nm-logging.h +++ b/src/nm-logging.h @@ -100,12 +100,20 @@ typedef enum { /*< skip >*/ #define nm_log_dbg(domain, ...) nm_log (LOGL_DEBUG, (domain), __VA_ARGS__) #define nm_log_trace(domain, ...) nm_log (LOGL_TRACE, (domain), __VA_ARGS__) +/* A wrapper for the _nm_log_impl() function that adds call site information. + * Contrary to nm_log(), it unconditionally calls the function without + * checking whether logging for the given level and domain is enabled. */ +#define _nm_log(level, domain, error, ...) \ + G_STMT_START { \ + _nm_log_impl (__FILE__, __LINE__, G_STRFUNC, (level), (domain), (error), ""__VA_ARGS__); \ + } G_STMT_END + /* nm_log() only evaluates it's argument list after checking * whether logging for the given level/domain is enabled. */ #define nm_log(level, domain, ...) \ G_STMT_START { \ if (nm_logging_enabled ((level), (domain))) { \ - _nm_log_impl (__FILE__, __LINE__, G_STRFUNC, (level), (domain), 0, ""__VA_ARGS__); \ + _nm_log (level, domain, 0, __VA_ARGS__); \ } \ } G_STMT_END From 65b074164e4f4244e39560015beaaf01defe0185 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 22 Apr 2015 11:12:29 +0200 Subject: [PATCH 7/7] logging: use _nm_log() to avoid duplicate check of whether logging is enabled Use _nm_log() in places that already checked whether logging is enabled. No need to check again as done by nm_log(). (cherry picked from commit 4526b55a516f2b7836a1879ca70d039b23dd7044) Conflicts: src/nm-route-manager.c --- src/nm-auth-manager.c | 6 +++--- src/nm-default-route-manager.c | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/nm-auth-manager.c b/src/nm-auth-manager.c index 2b929f6756..8bd167eff9 100644 --- a/src/nm-auth-manager.c +++ b/src/nm-auth-manager.c @@ -39,9 +39,9 @@ \ if ((self) != singleton_instance) \ g_snprintf (__prefix, sizeof (__prefix), "auth[%p]", (self)); \ - nm_log ((level), (domain), \ - "%s: " _NM_UTILS_MACRO_FIRST(__VA_ARGS__), \ - __prefix _NM_UTILS_MACRO_REST(__VA_ARGS__)); \ + _nm_log ((level), (domain), 0, \ + "%s: " _NM_UTILS_MACRO_FIRST(__VA_ARGS__), \ + __prefix _NM_UTILS_MACRO_REST(__VA_ARGS__)); \ } \ } G_STMT_END diff --git a/src/nm-default-route-manager.c b/src/nm-default-route-manager.c index 2fd396c4f0..153c1de6bb 100644 --- a/src/nm-default-route-manager.c +++ b/src/nm-default-route-manager.c @@ -76,9 +76,9 @@ static NMDefaultRouteManager *singleton_instance; g_snprintf (__prefix, sizeof (__prefix), "default-route%c[%p]", __ch, (self)); \ else \ __prefix[STRLEN ("default-route")] = __ch; \ - nm_log (__level, __domain, \ - "%s: " _NM_UTILS_MACRO_FIRST(__VA_ARGS__), \ - __prefix _NM_UTILS_MACRO_REST(__VA_ARGS__)); \ + _nm_log (__level, __domain, 0, \ + "%s: " _NM_UTILS_MACRO_FIRST(__VA_ARGS__), \ + __prefix _NM_UTILS_MACRO_REST(__VA_ARGS__)); \ } \ } G_STMT_END