From e18ff51d4f4eb073b5aa3dab1d762b5d8abbeb2f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 9 Jan 2019 21:58:26 +0100 Subject: [PATCH 1/4] tests: don't use alloca() in tests The only purpose of using alloca() to avoid the overhead of heap-allocation and possible save a line in source code for managing/freeing the heap allocation. For tests we don't care about performance, and (in this case) the code does not get any shorter. Avoid alloca() in tests, because alloca() is something to search for when reviewing code for stack overflows. No need to have such false positives show up in tests. --- src/tests/test-general.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tests/test-general.c b/src/tests/test-general.c index 2db6e60f6c..f065d12721 100644 --- a/src/tests/test-general.c +++ b/src/tests/test-general.c @@ -352,13 +352,13 @@ _match_connection (GSList *connections, gint64 default_v4_metric, gint64 default_v6_metric) { - NMConnection **list; + gs_free NMConnection **list = NULL; guint i, len; len = g_slist_length (connections); g_assert (len < 10); - list = g_alloca ((len + 1) * sizeof (NMConnection *)); + list = g_malloc ((len + 1) * sizeof (NMConnection *)); for (i = 0; i < len; i++, connections = connections->next) { g_assert (connections); g_assert (connections->data); From 617bdbd8c274caea00c964315b7e7c2db32febf9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 10 Jan 2019 09:17:28 +0100 Subject: [PATCH 2/4] all/trivial: rename NM_UTILS_LOOKUP_STR() to have "_A" suffix NM_UTILS_LOOKUP_STR() uses alloca(). Partly to avoid the overhead of malloc(), but more important because it's convenient to use. It does not require to declare a varible to manage the lifetime of the heap allocation. It's quite safe, because the stack allocation is of a fixed size of only a few bytes. Overall, I think the convenience that we get (resulting in simpler code) outweighs the danger of stack allocation in this case. It's still worth it. However, as it uses alloca(), it still must not be used inside a (unbound) loop and it is obviously a macro. Rename the macros to have a _A() suffix. This should make the peculiarities more apparent. --- shared/nm-utils/nm-macros-internal.h | 2 +- src/devices/nm-acd-manager.c | 4 ++-- src/devices/nm-device.c | 23 +++++++++++------------ src/nm-active-connection.c | 13 +++++++------ src/nm-policy.c | 4 ++-- src/vpn/nm-vpn-connection.c | 17 ++++++++++------- 6 files changed, 33 insertions(+), 30 deletions(-) diff --git a/shared/nm-utils/nm-macros-internal.h b/shared/nm-utils/nm-macros-internal.h index b993b3840c..1826919629 100644 --- a/shared/nm-utils/nm-macros-internal.h +++ b/shared/nm-utils/nm-macros-internal.h @@ -1194,7 +1194,7 @@ fcn_name (lookup_type val) \ /* Call the string-lookup-table function @fcn_name. If the function returns * %NULL, the numeric index is converted to string using a alloca() buffer. * Beware: this macro uses alloca(). */ -#define NM_UTILS_LOOKUP_STR(fcn_name, idx) \ +#define NM_UTILS_LOOKUP_STR_A(fcn_name, idx) \ ({ \ typeof (idx) _idx = (idx); \ const char *_s; \ diff --git a/src/devices/nm-acd-manager.c b/src/devices/nm-acd-manager.c index 3c6128f060..a0a175bec7 100644 --- a/src/devices/nm-acd-manager.c +++ b/src/devices/nm-acd-manager.c @@ -93,7 +93,7 @@ _acd_event_to_string (unsigned int event) return NULL; } -#define acd_event_to_string(event) NM_UTILS_LOOKUP_STR (_acd_event_to_string, event) +#define acd_event_to_string_a(event) NM_UTILS_LOOKUP_STR_A (_acd_event_to_string, event) static const char * acd_error_to_string (int error) @@ -201,7 +201,7 @@ acd_event (GIOChannel *source, GIOCondition condition, gpointer data) nm_platform_link_get_name (NM_PLATFORM_GET, self->ifindex)); break; default: - _LOGD ("unhandled event '%s'", acd_event_to_string (event->event)); + _LOGD ("unhandled event '%s'", acd_event_to_string_a (event->event)); break; } diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index f03dd1e59c..9ded911cc6 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -744,8 +744,7 @@ NM_UTILS_LOOKUP_STR_DEFINE (nm_device_state_reason_to_str, NMDeviceStateReason, NM_UTILS_LOOKUP_STR_ITEM (NM_DEVICE_STATE_REASON_SRIOV_CONFIGURATION_FAILED, "sriov-configuration-failed"), ); -#define reason_to_string(reason) \ - NM_UTILS_LOOKUP_STR (nm_device_state_reason_to_str, reason) +#define reason_to_string_a(reason) NM_UTILS_LOOKUP_STR_A (nm_device_state_reason_to_str, reason) NM_UTILS_LOOKUP_STR_DEFINE_STATIC (mtu_source_to_str, NMDeviceMtuSource, NM_UTILS_LOOKUP_DEFAULT_NM_ASSERT ("unknown"), @@ -13363,7 +13362,7 @@ _set_unmanaged_flags (NMDevice *self, flags, NM_PRINT_FMT_QUOTED (allow_state_transition, ", reason ", - reason_to_string (reason), + reason_to_string_a (reason), transition_state ? ", transition-state" : "", "")); @@ -14313,7 +14312,7 @@ nm_device_cleanup (NMDevice *self, NMDeviceStateReason reason, CleanupType clean if (reason == NM_DEVICE_STATE_REASON_NOW_MANAGED) _LOGD (LOGD_DEVICE, "preparing device"); else - _LOGD (LOGD_DEVICE, "deactivating device (reason '%s') [%d]", reason_to_string (reason), reason); + _LOGD (LOGD_DEVICE, "deactivating device (reason '%s') [%d]", reason_to_string_a (reason), reason); /* Save whether or not we tried IPv6 for later */ priv = NM_DEVICE_GET_PRIVATE (self); @@ -14700,7 +14699,7 @@ _set_state_full (NMDevice *self, _LOGD (LOGD_DEVICE, "state change: %s -> %s (reason '%s', sys-iface-state: '%s'%s)", nm_device_state_to_str (old_state), nm_device_state_to_str (state), - reason_to_string (reason), + reason_to_string_a (reason), _sys_iface_state_to_str (priv->sys_iface_state), priv->firmware_missing ? ", missing firmware" : ""); return; @@ -14709,7 +14708,7 @@ _set_state_full (NMDevice *self, _LOGI (LOGD_DEVICE, "state change: %s -> %s (reason '%s', sys-iface-state: '%s')", nm_device_state_to_str (old_state), nm_device_state_to_str (state), - reason_to_string (reason), + reason_to_string_a (reason), _sys_iface_state_to_str (priv->sys_iface_state)); priv->in_state_changed = TRUE; @@ -15046,7 +15045,7 @@ queued_state_set (gpointer user_data) _LOGD (LOGD_DEVICE, "queue-state[%s, reason:%s, id:%u]: %s", nm_device_state_to_str (priv->queued_state.state), - reason_to_string (priv->queued_state.reason), + reason_to_string_a (priv->queued_state.reason), priv->queued_state.id, "change state"); @@ -15077,11 +15076,11 @@ nm_device_queue_state (NMDevice *self, if (priv->queued_state.id && priv->queued_state.state == state) { _LOGD (LOGD_DEVICE, "queue-state[%s, reason:%s, id:%u]: %s%s%s%s", nm_device_state_to_str (priv->queued_state.state), - reason_to_string (priv->queued_state.reason), + reason_to_string_a (priv->queued_state.reason), priv->queued_state.id, "ignore queuing same state change", NM_PRINT_FMT_QUOTED (priv->queued_state.reason != reason, - " (reason differs: ", reason_to_string (reason), ")", "")); + " (reason differs: ", reason_to_string_a (reason), ")", "")); return; } @@ -15093,7 +15092,7 @@ nm_device_queue_state (NMDevice *self, if (priv->queued_state.id) { _LOGW (LOGD_DEVICE, "queue-state[%s, reason:%s, id:%u]: %s", nm_device_state_to_str (priv->queued_state.state), - reason_to_string (priv->queued_state.reason), + reason_to_string_a (priv->queued_state.reason), priv->queued_state.id, "replace previously queued state change"); nm_clear_g_source (&priv->queued_state.id); @@ -15106,7 +15105,7 @@ nm_device_queue_state (NMDevice *self, _LOGD (LOGD_DEVICE, "queue-state[%s, reason:%s, id:%u]: %s", nm_device_state_to_str (state), - reason_to_string (reason), + reason_to_string_a (reason), priv->queued_state.id, "queue state change"); } @@ -15121,7 +15120,7 @@ queued_state_clear (NMDevice *self) _LOGD (LOGD_DEVICE, "queue-state[%s, reason:%s, id:%u]: %s", nm_device_state_to_str (priv->queued_state.state), - reason_to_string (priv->queued_state.reason), + reason_to_string_a (priv->queued_state.reason), priv->queued_state.id, "clear queued state change"); nm_clear_g_source (&priv->queued_state.id); diff --git a/src/nm-active-connection.c b/src/nm-active-connection.c index 966a274351..7b9f2cc701 100644 --- a/src/nm-active-connection.c +++ b/src/nm-active-connection.c @@ -161,7 +161,8 @@ NM_UTILS_LOOKUP_STR_DEFINE_STATIC (_state_to_string, NMActiveConnectionState, NM_UTILS_LOOKUP_STR_ITEM (NM_ACTIVE_CONNECTION_STATE_DEACTIVATING, "deactivating"), NM_UTILS_LOOKUP_STR_ITEM (NM_ACTIVE_CONNECTION_STATE_DEACTIVATED, "deactivated"), ); -#define state_to_string(state) NM_UTILS_LOOKUP_STR (_state_to_string, state) + +#define state_to_string_a(state) NM_UTILS_LOOKUP_STR_A (_state_to_string, state) /* the maximum required buffer size for _state_flags_to_string(). */ #define _NM_ACTIVATION_STATE_FLAG_TO_STRING_BUFSIZE (255) @@ -256,8 +257,8 @@ nm_active_connection_set_state (NMActiveConnection *self, return; _LOGD ("set state %s (was %s)", - state_to_string (new_state), - state_to_string (priv->state)); + state_to_string_a (new_state), + state_to_string_a (priv->state)); if (new_state > NM_ACTIVE_CONNECTION_STATE_ACTIVATED) { /* once we are about to deactivate, we don't need the keep-alive instance @@ -786,11 +787,11 @@ check_master_ready (NMActiveConnection *self) signalling ? "signal" : (priv->master_ready ? "already signalled" : "not signalling"), - state_to_string (priv->state), + state_to_string_a (priv->state), priv->master ? nm_sprintf_bufa (128, "master %p is in state %s", priv->master, - state_to_string (nm_active_connection_get_state (priv->master))) + state_to_string_a (nm_active_connection_get_state (priv->master))) : "no master"); if (signalling) { @@ -854,7 +855,7 @@ nm_active_connection_set_master (NMActiveConnection *self, NMActiveConnection *m _LOGD ("set master %p, %s, state %s", master, nm_active_connection_get_settings_connection_id (master), - state_to_string (nm_active_connection_get_state (master))); + state_to_string_a (nm_active_connection_get_state (master))); priv->master = g_object_ref (master); g_signal_connect (priv->master, diff --git a/src/nm-policy.c b/src/nm-policy.c index 4120227757..77fc929c10 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -1882,8 +1882,8 @@ device_state_changed (NMDevice *device, if (blocked_reason != NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NONE) { _LOGD (LOGD_DEVICE, "blocking autoconnect of connection '%s': %s", nm_settings_connection_get_id (sett_conn), - NM_UTILS_LOOKUP_STR (nm_device_state_reason_to_str, - nm_device_state_reason_check (reason))); + NM_UTILS_LOOKUP_STR_A (nm_device_state_reason_to_str, + nm_device_state_reason_check (reason))); nm_settings_connection_autoconnect_blocked_reason_set (sett_conn, blocked_reason, TRUE); } } diff --git a/src/vpn/nm-vpn-connection.c b/src/vpn/nm-vpn-connection.c index 26c9a7a5ea..6626f64a04 100644 --- a/src/vpn/nm-vpn-connection.c +++ b/src/vpn/nm-vpn-connection.c @@ -884,14 +884,15 @@ NM_UTILS_LOOKUP_STR_DEFINE_STATIC (_vpn_plugin_failure_to_string, NMVpnPluginFai NM_UTILS_LOOKUP_STR_ITEM (NM_VPN_PLUGIN_FAILURE_CONNECT_FAILED, "connect-failed"), NM_UTILS_LOOKUP_STR_ITEM (NM_VPN_PLUGIN_FAILURE_BAD_IP_CONFIG, "bad-ip-config"), ); -#define vpn_plugin_failure_to_string(failure) NM_UTILS_LOOKUP_STR (_vpn_plugin_failure_to_string, failure) + +#define vpn_plugin_failure_to_string_a(failure) NM_UTILS_LOOKUP_STR_A (_vpn_plugin_failure_to_string, failure) static void plugin_failed (NMVpnConnection *self, guint reason) { NMVpnConnectionPrivate *priv = NM_VPN_CONNECTION_GET_PRIVATE (self); - _LOGW ("VPN plugin: failed: %s (%d)", vpn_plugin_failure_to_string (reason), reason); + _LOGW ("VPN plugin: failed: %s (%d)", vpn_plugin_failure_to_string_a (reason), reason); switch (reason) { case NM_VPN_PLUGIN_FAILURE_LOGIN_FAILED: @@ -916,7 +917,8 @@ NM_UTILS_LOOKUP_STR_DEFINE_STATIC (_vpn_service_state_to_string, NMVpnServiceSta NM_UTILS_LOOKUP_STR_ITEM (NM_VPN_SERVICE_STATE_STOPPING, "stopping"), NM_UTILS_LOOKUP_STR_ITEM (NM_VPN_SERVICE_STATE_STOPPED, "stopped"), ); -#define vpn_service_state_to_string(state) NM_UTILS_LOOKUP_STR (_vpn_service_state_to_string, state) + +#define vpn_service_state_to_string_a(state) NM_UTILS_LOOKUP_STR_A (_vpn_service_state_to_string, state) NM_UTILS_LOOKUP_STR_DEFINE_STATIC (_vpn_state_to_string, VpnState, NM_UTILS_LOOKUP_DEFAULT (NULL), @@ -932,7 +934,8 @@ NM_UTILS_LOOKUP_STR_DEFINE_STATIC (_vpn_state_to_string, VpnState, NM_UTILS_LOOKUP_STR_ITEM (STATE_DISCONNECTED, "disconnected"), NM_UTILS_LOOKUP_STR_ITEM (STATE_FAILED, "failed"), ); -#define vpn_state_to_string(state) NM_UTILS_LOOKUP_STR (_vpn_state_to_string, state) + +#define vpn_state_to_string_a(state) NM_UTILS_LOOKUP_STR_A (_vpn_state_to_string, state) static void plugin_state_changed (NMVpnConnection *self, NMVpnServiceState new_service_state) @@ -941,7 +944,7 @@ plugin_state_changed (NMVpnConnection *self, NMVpnServiceState new_service_state NMVpnServiceState old_service_state = priv->service_state; _LOGI ("VPN plugin: state changed: %s (%d)", - vpn_service_state_to_string (new_service_state), new_service_state); + vpn_service_state_to_string_a (new_service_state), new_service_state); priv->service_state = new_service_state; if (new_service_state == NM_VPN_SERVICE_STATE_STOPPED) { @@ -2690,12 +2693,12 @@ plugin_interactive_secrets_required (NMVpnConnection *self, if (!NM_IN_SET (priv->vpn_state, STATE_CONNECT, STATE_NEED_AUTH)) { _LOGD ("VPN plugin: requested secrets; state %s (%d); ignore request in current state", - vpn_state_to_string (priv->vpn_state), priv->vpn_state); + vpn_state_to_string_a (priv->vpn_state), priv->vpn_state); return; } _LOGI ("VPN plugin: requested secrets; state %s (%d)", - vpn_state_to_string (priv->vpn_state), priv->vpn_state); + vpn_state_to_string_a (priv->vpn_state), priv->vpn_state); priv->secrets_idx = SECRETS_REQ_INTERACTIVE; _set_vpn_state (self, STATE_NEED_AUTH, NM_ACTIVE_CONNECTION_STATE_REASON_NONE, FALSE); From 3263cab5965755c29d54997c4f6b199f913e1db9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 10 Jan 2019 12:00:34 +0100 Subject: [PATCH 3/4] all: add static assertion for maximumg alloca() allocated buffer Add a compile time check that the buffer that we allocate on the stack is reasonably small. --- shared/nm-utils/nm-macros-internal.h | 32 +++++++++++++++------ shared/nm-utils/nm-shared-utils.h | 2 ++ shared/nm-utils/tests/test-shared-general.c | 2 ++ src/nm-logging.c | 30 +++++++++---------- src/platform/nm-platform.c | 13 +++++---- src/platform/nm-platform.h | 18 ++++++------ 6 files changed, 58 insertions(+), 39 deletions(-) diff --git a/shared/nm-utils/nm-macros-internal.h b/shared/nm-utils/nm-macros-internal.h index 1826919629..1f3970fe4a 100644 --- a/shared/nm-utils/nm-macros-internal.h +++ b/shared/nm-utils/nm-macros-internal.h @@ -1409,7 +1409,8 @@ nm_decode_version (guint version, guint *major, guint *minor, guint *micro) * If @str is longer then @trunc_at, the string is truncated and the closing * quote is instead '^' to indicate truncation. * - * Thus, the maximum stack allocated buffer will be @trunc_at+3. */ + * Thus, the maximum stack allocated buffer will be @trunc_at+3. The maximum + * buffer size must be a constant and not larger than 300. */ #define nm_strquote_a(trunc_at, str) \ ({ \ const char *const _str = (str); \ @@ -1420,6 +1421,8 @@ nm_decode_version (guint version, guint *major, guint *minor, guint *micro) const gsize _strlen_trunc = NM_MIN (strlen (_str), _trunc_at); \ char *_buf; \ \ + G_STATIC_ASSERT_EXPR ((trunc_at) <= 300); \ + \ _buf = g_alloca (_strlen_trunc + 3); \ _buf[0] = '"'; \ memcpy (&_buf[1], _str, _strlen_trunc); \ @@ -1444,19 +1447,30 @@ nm_decode_version (guint version, guint *major, guint *minor, guint *micro) _buf; \ }) -#define nm_sprintf_bufa(n_elements, format, ...) \ +/* it is "unsafe" because @bufsize must not be a constant expression and + * there is no check at compiletime. Regardless of that, the buffer size + * must not be larger than 300 bytes, as this gets stack allocated. */ +#define nm_sprintf_buf_unsafe_a(bufsize, format, ...) \ ({ \ char *_buf; \ int _buf_len; \ - typeof (n_elements) _n_elements = (n_elements); \ + typeof (bufsize) _bufsize = (bufsize); \ \ - _buf = g_alloca (_n_elements); \ - _buf_len = g_snprintf (_buf, _n_elements, \ + nm_assert (_bufsize <= 300); \ + \ + _buf = g_alloca (_bufsize); \ + _buf_len = g_snprintf (_buf, _bufsize, \ ""format"", ##__VA_ARGS__); \ - nm_assert (_buf_len < _n_elements); \ + nm_assert (_buf_len >= 0 && _buf_len < _bufsize); \ _buf; \ }) +#define nm_sprintf_bufa(bufsize, format, ...) \ + ({ \ + G_STATIC_ASSERT_EXPR ((bufsize) <= 300); \ + nm_sprintf_buf_unsafe_a ((bufsize), format, ##__VA_ARGS__); \ + }) + /* aims to alloca() a buffer and fill it with printf(format, name). * Note that format must not contain any format specifier except * "%s". @@ -1470,9 +1484,9 @@ nm_decode_version (guint version, guint *major, guint *minor, guint *micro) char *_buf2; \ \ nm_assert (_p_val_to_free && !*_p_val_to_free); \ - if ( NM_STRLEN (format) < 200 \ - && _name_len < (gsize) (200 - NM_STRLEN (format))) \ - _buf2 = nm_sprintf_bufa (NM_STRLEN (format) + _name_len, format, _name); \ + if ( NM_STRLEN (format) <= 290 \ + && _name_len < (gsize) (290 - NM_STRLEN (format))) \ + _buf2 = nm_sprintf_buf_unsafe_a (NM_STRLEN (format) + _name_len, format, _name); \ else { \ _buf2 = g_strdup_printf (format, _name); \ *_p_val_to_free = _buf2; \ diff --git a/shared/nm-utils/nm-shared-utils.h b/shared/nm-utils/nm-shared-utils.h index ac8fe51ec9..8dedac6580 100644 --- a/shared/nm-utils/nm-shared-utils.h +++ b/shared/nm-utils/nm-shared-utils.h @@ -278,6 +278,8 @@ _nm_strndup_a_step (char *s, const char *str, gsize len) char **const _out_str_free = (out_str_free); \ char *_s; \ \ + G_STATIC_ASSERT_EXPR ((alloca_maxlen) <= 300); \ + \ if ( _out_str_free \ && _len >= _alloca_maxlen) { \ _s = g_malloc (_len + 1); \ diff --git a/shared/nm-utils/tests/test-shared-general.c b/shared/nm-utils/tests/test-shared-general.c index ecc138748f..b10e2680f8 100644 --- a/shared/nm-utils/tests/test-shared-general.c +++ b/shared/nm-utils/tests/test-shared-general.c @@ -98,6 +98,8 @@ test_make_strv (void) G_STATIC_ASSERT_EXPR (G_N_ELEMENTS (NM_MAKE_STRV ("a", "b" )) == 3); G_STATIC_ASSERT_EXPR (G_N_ELEMENTS (NM_MAKE_STRV ("a", "b", )) == 3); + + nm_strquote_a (300, ""); } /*****************************************************************************/ diff --git a/src/nm-logging.c b/src/nm-logging.c index 7af64baefb..6f912f5862 100644 --- a/src/nm-logging.c +++ b/src/nm-logging.c @@ -42,17 +42,6 @@ #include "nm-utils/nm-time-utils.h" #include "nm-errors.h" -/* often we have some static string where we need to know the maximum length. - * _MAX_LEN() returns @max but adds a debugging assertion that @str is indeed - * shorter then @mac. */ -#define _MAX_LEN(max, str) \ - ({ \ - const char *const _str = (str); \ - \ - nm_assert (_str && strlen (str) < (max)); \ - (max); \ - }) - void (*_nm_logging_clear_platform_logging_cache) (void); static void @@ -592,14 +581,25 @@ _iovec_set_format (struct iovec *iov, gpointer *iov_free, const char *format, .. char *const _buf = g_alloca (_size); \ int _len; \ \ + G_STATIC_ASSERT_EXPR ((reserve_extra) + (NM_STRLEN (format) + 3) <= 96); \ + \ _len = g_snprintf (_buf, _size, ""format"", ##__VA_ARGS__);\ \ nm_assert (_len >= 0); \ - nm_assert (_len <= _size); \ + nm_assert (_len < _size); \ nm_assert (_len == strlen (_buf)); \ \ _iovec_set ((iov), _buf, _len); \ } G_STMT_END + +#define _iovec_set_format_str_a(iov, max_str_len, format, str_arg) \ + G_STMT_START { \ + const char *_str_arg = (str_arg); \ + \ + nm_assert (_str_arg && strlen (_str_arg) < (max_str_len)); \ + _iovec_set_format_a ((iov), (max_str_len), format, str_arg); \ + } G_STMT_END + #endif void @@ -699,7 +699,7 @@ _nm_log_impl (const char *file, if (NM_FLAGS_ANY (dom, diter->num)) { if (i_domain > 0) { /* SYSLOG_FACILITY is specified multiple times for each domain that is actually enabled. */ - _iovec_set_format_a (iov++, _MAX_LEN (30, diter->name), "SYSLOG_FACILITY=%s", diter->name); + _iovec_set_format_str_a (iov++, 30, "SYSLOG_FACILITY=%s", diter->name); i_domain--; } dom &= ~diter->num; @@ -710,9 +710,9 @@ _nm_log_impl (const char *file, if (s_domain_all) _iovec_set (iov++, s_domain_all->str, s_domain_all->len); else - _iovec_set_format_a (iov++, _MAX_LEN (30, s_domain_1), "NM_LOG_DOMAINS=%s", s_domain_1); + _iovec_set_format_str_a (iov++, 30, "NM_LOG_DOMAINS=%s", s_domain_1); } - _iovec_set_format_a (iov++, _MAX_LEN (15, global.level_desc[level].name), "NM_LOG_LEVEL=%s", global.level_desc[level].name); + _iovec_set_format_str_a (iov++, 15, "NM_LOG_LEVEL=%s", global.level_desc[level].name); if (func) _iovec_set_format (iov++, iov_free++, "CODE_FUNC=%s", func); _iovec_set_format (iov++, iov_free++, "CODE_FILE=%s", file ?: ""); diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c index d097186ef7..bebb6ad3dc 100644 --- a/src/platform/nm-platform.c +++ b/src/platform/nm-platform.c @@ -2293,9 +2293,10 @@ link_set_option (NMPlatform *self, int ifindex, const char *category, const char if (dirfd < 0) return FALSE; - path = nm_sprintf_bufa (strlen (category) + strlen (option) + 2, - "%s/%s", - category, option); + path = nm_sprintf_buf_unsafe_a (strlen (category) + strlen (option) + 2, + "%s/%s", + category, + option); return nm_platform_sysctl_set (self, NMP_SYSCTL_PATHID_NETDIR_unsafe (dirfd, ifname_verified, path), value); } @@ -2313,9 +2314,9 @@ link_get_option (NMPlatform *self, int ifindex, const char *category, const char if (dirfd < 0) return NULL; - path = nm_sprintf_bufa (strlen (category) + strlen (option) + 2, - "%s/%s", - category, option); + path = nm_sprintf_buf_unsafe_a (strlen (category) + strlen (option) + 2, + "%s/%s", + category, option); return nm_platform_sysctl_get (self, NMP_SYSCTL_PATHID_NETDIR_unsafe (dirfd, ifname_verified, path)); } diff --git a/src/platform/nm-platform.h b/src/platform/nm-platform.h index 9bfc4bc398..1f136da582 100644 --- a/src/platform/nm-platform.h +++ b/src/platform/nm-platform.h @@ -1087,15 +1087,15 @@ const char *nm_link_type_to_string (NMLinkType link_type); ((const char *) NULL), -1, (path) #define NMP_SYSCTL_PATHID_NETDIR_unsafe(dirfd, ifname, path) \ - nm_sprintf_bufa ( NM_STRLEN ("net:/sys/class/net//\0") \ - + NMP_IFNAMSIZ \ - + ({ \ - const gsize _l = strlen (path); \ - \ - nm_assert (_l < 200); \ - _l; \ - }), \ - "net:/sys/class/net/%s/%s", (ifname), (path)), \ + nm_sprintf_buf_unsafe_a ( NM_STRLEN ("net:/sys/class/net//\0") \ + + NMP_IFNAMSIZ \ + + ({ \ + const gsize _l = strlen (path); \ + \ + nm_assert (_l < 200); \ + _l; \ + }), \ + "net:/sys/class/net/%s/%s", (ifname), (path)), \ (dirfd), (path) #define NMP_SYSCTL_PATHID_NETDIR(dirfd, ifname, path) \ From 8c2d58b23746babf021e546449ea7b1c7549f6ba Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 10 Jan 2019 12:38:16 +0100 Subject: [PATCH 4/4] shared/tests: add test for nm_strdup_int() macro --- shared/nm-utils/nm-shared-utils.h | 2 + shared/nm-utils/tests/test-shared-general.c | 49 +++++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/shared/nm-utils/nm-shared-utils.h b/shared/nm-utils/nm-shared-utils.h index 8dedac6580..66d2b84e8e 100644 --- a/shared/nm-utils/nm-shared-utils.h +++ b/shared/nm-utils/nm-shared-utils.h @@ -304,6 +304,8 @@ _nm_strndup_a_step (char *s, const char *str, gsize len) #if _NM_CC_SUPPORT_GENERIC #define nm_strdup_int(val) \ _Generic ((val), \ + char: g_strdup_printf ("%d", (int) (val)), \ + \ gint8: g_strdup_printf ("%d", (int) (val)), \ gint16: g_strdup_printf ("%d", (int) (val)), \ gint32: g_strdup_printf ("%d", (int) (val)), \ diff --git a/shared/nm-utils/tests/test-shared-general.c b/shared/nm-utils/tests/test-shared-general.c index b10e2680f8..d7df46dc52 100644 --- a/shared/nm-utils/tests/test-shared-general.c +++ b/shared/nm-utils/tests/test-shared-general.c @@ -104,6 +104,54 @@ test_make_strv (void) /*****************************************************************************/ +typedef enum { + TEST_NM_STRDUP_ENUM_m1 = -1, + TEST_NM_STRDUP_ENUM_3 = 3, +} TestNMStrdupIntEnum; + +static void +test_nm_strdup_int (void) +{ +#define _NM_STRDUP_INT_TEST(num, str) \ + G_STMT_START { \ + gs_free char *_s1 = NULL; \ + \ + _s1 = nm_strdup_int ((num)); \ + \ + g_assert (_s1); \ + g_assert_cmpstr (_s1, ==, str); \ + } G_STMT_END + +#define _NM_STRDUP_INT_TEST_TYPED(type, num) \ + G_STMT_START { \ + type _num = ((type) num); \ + \ + _NM_STRDUP_INT_TEST (_num, G_STRINGIFY (num)); \ + } G_STMT_END + + _NM_STRDUP_INT_TEST_TYPED (char, 0); + _NM_STRDUP_INT_TEST_TYPED (char, 1); + _NM_STRDUP_INT_TEST_TYPED (guint8, 0); + _NM_STRDUP_INT_TEST_TYPED (gint8, 25); + _NM_STRDUP_INT_TEST_TYPED (char, 47); + _NM_STRDUP_INT_TEST_TYPED (short, 47); + _NM_STRDUP_INT_TEST_TYPED (int, 47); + _NM_STRDUP_INT_TEST_TYPED (long, 47); + _NM_STRDUP_INT_TEST_TYPED (unsigned char, 47); + _NM_STRDUP_INT_TEST_TYPED (unsigned short, 47); + _NM_STRDUP_INT_TEST_TYPED (unsigned, 47); + _NM_STRDUP_INT_TEST_TYPED (unsigned long, 47); + _NM_STRDUP_INT_TEST_TYPED (gint64, 9223372036854775807); + _NM_STRDUP_INT_TEST_TYPED (gint64, -9223372036854775807); + _NM_STRDUP_INT_TEST_TYPED (guint64, 0); + _NM_STRDUP_INT_TEST_TYPED (guint64, 9223372036854775807); + + _NM_STRDUP_INT_TEST (TEST_NM_STRDUP_ENUM_m1, "-1"); + _NM_STRDUP_INT_TEST (TEST_NM_STRDUP_ENUM_3, "3"); +} + +/*****************************************************************************/ + NMTST_DEFINE (); int main (int argc, char **argv) @@ -113,6 +161,7 @@ int main (int argc, char **argv) g_test_add_func ("/general/test_monotonic_timestamp", test_monotonic_timestamp); g_test_add_func ("/general/test_nmhash", test_nmhash); g_test_add_func ("/general/test_nm_make_strv", test_make_strv); + g_test_add_func ("/general/test_nm_strdup_int", test_nm_strdup_int); return g_test_run (); }