From 9cddb9f8bd77325935296e75cbc03499cfa0a1b7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 14 May 2019 12:56:44 +0200 Subject: [PATCH] logging: use static buffer in nm_logging_all_domains_to_string() Don't create a heap allocated GString to hold the static result of nm_logging_all_domains_to_string(). Instead, use a static buffer of the exactly required size. The main reason to do this, is to get the exact size of "_all_logging_domains_to_str" buffer. This is the upper boundary for the size of a string buffer to hold all domain names. We will need that boundary in the next commit. The attractive thing here is that we will have a unit-test failure if this boundery no longer matches (--with-more-asserts). That means, this boundary is guarded by unit tests and we don't accidentally get it wrong when the domains change. Also, take care to initialize the buffer in a thread-safe manner. It's easy enough to get right, so there is no excuse for having non-thread-safe code in logging. --- src/nm-logging.c | 41 +++++++++++++++++++++++++++++++--------- src/tests/test-general.c | 13 +++++++++++++ 2 files changed, 45 insertions(+), 9 deletions(-) diff --git a/src/nm-logging.c b/src/nm-logging.c index 4fea7595f7..eeffeb657f 100644 --- a/src/nm-logging.c +++ b/src/nm-logging.c @@ -549,27 +549,50 @@ _domains_to_string (gboolean include_level_override, return g_string_free (str, FALSE); } +static char _all_logging_domains_to_str[273]; + const char * nm_logging_all_domains_to_string (void) { - static GString *str; + static const char *volatile str = NULL; + const char *s; - if (G_UNLIKELY (!str)) { +again: + s = g_atomic_pointer_get (&str); + if (G_UNLIKELY (!s)) { + static gsize once = 0; const LogDesc *diter; + gsize buf_l; + char *buf_p; - str = g_string_new (LOGD_DEFAULT_STRING); + if (!g_once_init_enter (&once)) + goto again; + + buf_p = _all_logging_domains_to_str; + buf_l = sizeof (_all_logging_domains_to_str); + + nm_utils_strbuf_append_str (&buf_p, &buf_l, LOGD_DEFAULT_STRING); for (diter = &domain_desc[0]; diter->name; diter++) { - g_string_append_c (str, ','); - g_string_append (str, diter->name); + nm_utils_strbuf_append_c (&buf_p, &buf_l, ','); + nm_utils_strbuf_append_str (&buf_p, &buf_l, diter->name); if (diter->num == LOGD_DHCP6) - g_string_append (str, "," LOGD_DHCP_STRING); + nm_utils_strbuf_append_str (&buf_p, &buf_l, ","LOGD_DHCP_STRING); else if (diter->num == LOGD_IP6) - g_string_append (str, "," LOGD_IP_STRING); + nm_utils_strbuf_append_str (&buf_p, &buf_l, ","LOGD_IP_STRING); } - g_string_append (str, "," LOGD_ALL_STRING); + nm_utils_strbuf_append_str (&buf_p, &buf_l, LOGD_ALL_STRING); + + /* Did you modify the logging domains (or their names)? Adjust the size of + * _all_logging_domains_to_str buffer above to have the exact size. */ + nm_assert (strlen (_all_logging_domains_to_str) == sizeof (_all_logging_domains_to_str) - 1); + nm_assert (buf_l == 1); + + s = _all_logging_domains_to_str; + g_atomic_pointer_set (&str, s); + g_once_init_leave (&once, 1); } - return str->str; + return s; } /** diff --git a/src/tests/test-general.c b/src/tests/test-general.c index 0dee566eaa..cedf5b8a9a 100644 --- a/src/tests/test-general.c +++ b/src/tests/test-general.c @@ -108,6 +108,17 @@ test_nm_utils_ip6_address_clear_host_address (void) /*****************************************************************************/ +static void +test_logging_domains (void) +{ + const char *s; + + s = nm_logging_all_domains_to_string (); + g_assert (s && s[0]); +} + +/*****************************************************************************/ + static void _test_same_prefix (const char *a1, const char *a2, guint8 plen) { @@ -2180,6 +2191,8 @@ main (int argc, char **argv) { nmtst_init_with_logging (&argc, &argv, NULL, "ALL"); + g_test_add_func ("/general/test_logging_domains", test_logging_domains); + g_test_add_func ("/general/nm_utils_strbuf_append", test_nm_utils_strbuf_append); g_test_add_func ("/general/nm_utils_ip6_address_clear_host_address", test_nm_utils_ip6_address_clear_host_address);