From 5ff1468717e558038adfb6b508446f83fe9edacc Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 28 Jul 2023 11:43:49 +0200 Subject: [PATCH 1/4] all: ensure signendess for arguments of NM_{MIN,MAX,CLAMP}() macros matches --- src/core/devices/nm-device-wireguard.c | 2 +- src/core/devices/nm-device.c | 8 ++++---- src/core/dhcp/nm-dhcp-helper.c | 4 ++-- src/core/dnsmasq/nm-dnsmasq-utils.c | 4 ++-- src/core/nm-manager.c | 2 +- src/core/platform/tests/test-common.h | 2 +- src/core/tests/test-l3cfg.c | 2 +- src/libnm-lldp/nm-lldp-rx.c | 2 +- src/libnm-platform/nm-linux-platform.c | 2 +- src/libnm-platform/nm-platform.c | 4 ++-- src/nm-initrd-generator/nmi-cmdline-reader.c | 2 +- 11 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/core/devices/nm-device-wireguard.c b/src/core/devices/nm-device-wireguard.c index db02033d05..df84e77bad 100644 --- a/src/core/devices/nm-device-wireguard.c +++ b/src/core/devices/nm-device-wireguard.c @@ -720,7 +720,7 @@ _peers_retry_in_msec(PeerData *peer_data, gboolean after_failure) return RETRY_IN_MSEC_MAX; /* double the retry-time, starting with one second. */ - return NM_MIN(RETRY_IN_MSEC_MAX, (1u << peer_data->ep_resolv.resolv_fail_count) * 500); + return NM_MIN(RETRY_IN_MSEC_MAX, (1l << peer_data->ep_resolv.resolv_fail_count) * 500); } static void diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index 2038e2f205..5f1b057a38 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -5701,7 +5701,7 @@ out: return FALSE; } -#define CONCHECK_P_PROBE_INTERVAL 1 +#define CONCHECK_P_PROBE_INTERVAL 1u static void concheck_periodic_schedule_set(NMDevice *self, int addr_family, ConcheckScheduleMode mode) @@ -5873,7 +5873,7 @@ concheck_update_interval(NMDevice *self, int addr_family, gboolean check_now) new_interval = nm_connectivity_get_interval(concheck_get_mgr(self)); - new_interval = NM_MIN(new_interval, 7 * 24 * 3600); + new_interval = NM_MIN(new_interval, 7u * 24u * 3600u); if (new_interval != priv->concheck_x[IS_IPv4].p_max_interval) { _LOGT(LOGD_CONCHECK, @@ -11546,13 +11546,13 @@ _commit_mtu(NMDevice *self) mtu_plat = nm_platform_link_get_mtu(nm_device_get_platform(self), ifindex); if (ip6_mtu) { - ip6_mtu = NM_MAX(1280, ip6_mtu); + ip6_mtu = NM_MAX(1280u, ip6_mtu); if (!mtu_desired) mtu_desired = mtu_plat; if (mtu_desired) { - mtu_desired = NM_MAX(1280, mtu_desired); + mtu_desired = NM_MAX(1280u, mtu_desired); if (mtu_desired < ip6_mtu) ip6_mtu = mtu_desired; diff --git a/src/core/dhcp/nm-dhcp-helper.c b/src/core/dhcp/nm-dhcp-helper.c index ee95abb70b..9e4cedf2c8 100644 --- a/src/core/dhcp/nm-dhcp-helper.c +++ b/src/core/dhcp/nm-dhcp-helper.c @@ -159,7 +159,7 @@ do_connect: error->message, try_count, (long long) (time_end - remaining_time - time_start) / 1000); - interval = NM_CLAMP((gint64) (100L * (1L << NM_MIN(try_count, 31))), 5000, 100000); + interval = NM_CLAMP((gint64) (100L * (1L << NM_MIN(try_count, 31u))), 5000, 100000); g_usleep(NM_MIN(interval, remaining_time)); g_clear_error(&error); goto do_connect; @@ -222,7 +222,7 @@ do_notify: gint64 interval; _LOGi("failure to call notify: %s (retry %u)", error->message, try_count); - interval = NM_CLAMP((gint64) (100L * (1L << NM_MIN(try_count, 31))), 5000, 25000); + interval = NM_CLAMP((gint64) (100L * (1L << NM_MIN(try_count, 31u))), 5000, 25000); g_usleep(NM_MIN(interval, remaining_time)); g_clear_error(&error); goto do_notify; diff --git a/src/core/dnsmasq/nm-dnsmasq-utils.c b/src/core/dnsmasq/nm-dnsmasq-utils.c index 69424d74a6..8d15c2e3d2 100644 --- a/src/core/dnsmasq/nm-dnsmasq-utils.c +++ b/src/core/dnsmasq/nm-dnsmasq-utils.c @@ -83,12 +83,12 @@ nm_dnsmasq_utils_get_range(const NMPlatformIP4Address *addr, mid = (host & netmask) | (((first + last) / 2) & ~netmask); if (host > mid) { /* use lower range */ - reserved = NM_MIN(((host - first) / 10), 8); + reserved = NM_MIN(((host - first) / 10u), 8u); last = host - 1 - reserved; first = NM_MAX(first, last > NUM ? last - NUM : 0); } else { /* use upper range */ - reserved = NM_MIN(((last - host) / 10), 8); + reserved = NM_MIN(((last - host) / 10u), 8u); first = host + 1 + reserved; last = NM_MIN(last, first < 0xFFFFFFFF - NUM ? first + NUM : 0xFFFFFFFF); } diff --git a/src/core/nm-manager.c b/src/core/nm-manager.c index 9c7212202b..2175a9c48f 100644 --- a/src/core/nm-manager.c +++ b/src/core/nm-manager.c @@ -800,7 +800,7 @@ initited: * a lot of stale entries. We must from time to time clean them up. * * Do do this cleanup, whenever we have more entries then 2 times the number of links. */ - if (G_UNLIKELY(g_hash_table_size(priv->device_route_metrics) > NM_MAX(20, n_links * 2))) { + if (G_UNLIKELY(g_hash_table_size(priv->device_route_metrics) > NM_MAX(20u, n_links * 2u))) { /* from time to time, we need to do some house-keeping and prune stale entries. * Otherwise, on a system where interfaces frequently come and go (docker), we * keep growing this cache for ifindexes that no longer exist. */ diff --git a/src/core/platform/tests/test-common.h b/src/core/platform/tests/test-common.h index 2802d8fda0..12d6e7ce67 100644 --- a/src/core/platform/tests/test-common.h +++ b/src/core/platform/tests/test-common.h @@ -32,7 +32,7 @@ * Normalize the requested_value to the kernel_value, if it look as if a rounding * error happens. If the difference is larger than +/- 1, no normalization happens! */ \ \ - ((_requested_value >= (NM_MAX(_kernel_value, 1) - 1)) \ + ((_requested_value >= (NM_MAX(_kernel_value, (typeof(_kernel_value)) 1) - 1)) \ && (_requested_value <= (NM_MIN(_kernel_value, ~((typeof(_kernel_value)) 0) - 1) + 1))) \ ? _kernel_value \ : _requested_value; \ diff --git a/src/core/tests/test-l3cfg.c b/src/core/tests/test-l3cfg.c index 962b2eecdf..6b82fe433e 100644 --- a/src/core/tests/test-l3cfg.c +++ b/src/core/tests/test-l3cfg.c @@ -760,7 +760,7 @@ test_l3_ipv4ll(gconstpointer test_data) if (next_timeout_msec <= 0) break; - next_timeout_msec = NM_MIN(next_timeout_msec, nmtst_get_rand_uint32() % 1000u); + next_timeout_msec = NM_MIN(next_timeout_msec, (gint64) (nmtst_get_rand_uint32() % 1000u)); nmtst_main_context_iterate_until(NULL, next_timeout_msec, FALSE); _LOGT("poll 1 intermezzo"); diff --git a/src/libnm-lldp/nm-lldp-rx.c b/src/libnm-lldp/nm-lldp-rx.c index 6d0f4a184e..345c6d5661 100644 --- a/src/libnm-lldp/nm-lldp-rx.c +++ b/src/libnm-lldp/nm-lldp-rx.c @@ -365,7 +365,7 @@ lldp_rx_start_timer(NMLldpRX *lldp_rx, NMLldpNeighbor *neighbor) timeout_msec = (n->until_usec / 1000) - nm_utils_get_monotonic_timestamp_msec(); lldp_rx->timer_event_source = - nm_g_source_attach(nm_g_timeout_source_new(NM_CLAMP(timeout_msec, 0, G_MAXUINT), + nm_g_source_attach(nm_g_timeout_source_new(NM_CLAMP(timeout_msec, 0, (gint64) G_MAXUINT), G_PRIORITY_DEFAULT, on_timer_event, lldp_rx, diff --git a/src/libnm-platform/nm-linux-platform.c b/src/libnm-platform/nm-linux-platform.c index 99eab9c784..87c5ce5781 100644 --- a/src/libnm-platform/nm-linux-platform.c +++ b/src/libnm-platform/nm-linux-platform.c @@ -3922,7 +3922,7 @@ _new_from_nl_route(const struct nlmsghdr *nlh, gboolean id_only, ParseNlmsgIter * hops in this list). */ nm_assert(v4_n_nexthops > 0u); if (v4_n_nexthops - 1u >= v4_nh_extra_alloc) { - v4_nh_extra_alloc = NM_MAX(4, v4_nh_extra_alloc * 2u); + v4_nh_extra_alloc = NM_MAX(4u, v4_nh_extra_alloc * 2u); if (!v4_nh_extra_nexthops_heap) { v4_nh_extra_nexthops_heap = g_new(NMPlatformIP4RtNextHop, v4_nh_extra_alloc); diff --git a/src/libnm-platform/nm-platform.c b/src/libnm-platform/nm-platform.c index 041354cf44..2f55eb94e9 100644 --- a/src/libnm-platform/nm-platform.c +++ b/src/libnm-platform/nm-platform.c @@ -719,7 +719,7 @@ nm_platform_sysctl_ip_neigh_set_ipv6_reachable_time(NMPlatform *self, /* RFC 4861 says the value can't be greater than one hour. * Also use a reasonable lower threshold. */ - clamped = NM_CLAMP(value_ms, 100, 3600000); + clamped = NM_CLAMP(value_ms, 100u, 3600000u); nm_sprintf_buf(path, "/proc/sys/net/ipv6/neigh/%s/base_reachable_time_ms", iface); nm_sprintf_buf(str, "%u", clamped); if (!nm_platform_sysctl_set(self, NMP_SYSCTL_PATHID_ABSOLUTE(path), str)) @@ -746,7 +746,7 @@ nm_platform_sysctl_ip_neigh_set_ipv6_retrans_time(NMPlatform *self, return TRUE; nm_sprintf_buf(path, "/proc/sys/net/ipv6/neigh/%s/retrans_time_ms", iface); - nm_sprintf_buf(str, "%u", NM_CLAMP(value_ms, 10, 3600000)); + nm_sprintf_buf(str, "%u", NM_CLAMP(value_ms, 10u, 3600000u)); return nm_platform_sysctl_set(self, NMP_SYSCTL_PATHID_ABSOLUTE(path), str); } diff --git a/src/nm-initrd-generator/nmi-cmdline-reader.c b/src/nm-initrd-generator/nmi-cmdline-reader.c index 68fe7004e6..89abed7d5d 100644 --- a/src/nm-initrd-generator/nmi-cmdline-reader.c +++ b/src/nm-initrd-generator/nmi-cmdline-reader.c @@ -1435,7 +1435,7 @@ nmi_cmdline_reader_parse(const char *etc_connections_dir, } } - reader->dhcp_timeout = NM_CLAMP(dhcp_timeout * dhcp_num_tries, 1, G_MAXINT32); + reader->dhcp_timeout = NM_CLAMP(dhcp_timeout * dhcp_num_tries, 1u, (guint32) G_MAXINT32); for (i = 0; argv[i]; i++) { gs_free char *argument_clone = NULL; From 3732f083191184d4e4ff1f6232e12f5018b016ba Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 28 Jul 2023 11:33:38 +0200 Subject: [PATCH 2/4] std-aux: add _NM_INT_IS_SIGNED() and _NM_INT_SAME_SIGNEDNESS() macros --- src/libnm-glib-aux/tests/test-shared-general.c | 7 +++++++ src/libnm-std-aux/nm-std-aux.h | 6 ++++++ 2 files changed, 13 insertions(+) diff --git a/src/libnm-glib-aux/tests/test-shared-general.c b/src/libnm-glib-aux/tests/test-shared-general.c index 84d87d4848..4a2dec0b26 100644 --- a/src/libnm-glib-aux/tests/test-shared-general.c +++ b/src/libnm-glib-aux/tests/test-shared-general.c @@ -32,6 +32,13 @@ G_STATIC_ASSERT(_nm_alignof(NMEtherAddr) <= _nm_alignof(NMIPAddr)); /*****************************************************************************/ +G_STATIC_ASSERT(_NM_INT_IS_SIGNED(1)); +G_STATIC_ASSERT(!_NM_INT_IS_SIGNED(1u)); +G_STATIC_ASSERT(_NM_INT_SAME_SIGNEDNESS((short) 1, 1l)); +G_STATIC_ASSERT(!_NM_INT_SAME_SIGNEDNESS((unsigned short) 1, 1l)); + +/*****************************************************************************/ + static void test_nm_static_assert(void) { diff --git a/src/libnm-std-aux/nm-std-aux.h b/src/libnm-std-aux/nm-std-aux.h index d310305e24..e65f2c417a 100644 --- a/src/libnm-std-aux/nm-std-aux.h +++ b/src/libnm-std-aux/nm-std-aux.h @@ -154,6 +154,12 @@ typedef uint64_t _nm_bitwise nm_be64_t; /*****************************************************************************/ +#define _NM_INT_IS_SIGNED(arg) (!(((typeof(arg)) -1) > 0)) + +#define _NM_INT_SAME_SIGNEDNESS(arg1, arg2) (_NM_INT_IS_SIGNED(arg1) == _NM_INT_IS_SIGNED(arg2)) + +/*****************************************************************************/ + #define NM_PASTE_ARGS(identifier1, identifier2) identifier1##identifier2 #define NM_PASTE(identifier1, identifier2) NM_PASTE_ARGS(identifier1, identifier2) From f27bf0b9ea83186d5b8104bb48182b2000a62c28 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 28 Jul 2023 11:38:33 +0200 Subject: [PATCH 3/4] std-aux: add static-asserts about signedness of NM_MIN()/NM_MAX()/NM_CLAMP() The macros NM_MIN()/NM_MAX()/NM_CLAMP() use typeof() to accept any integer type as argument. Internally, they rely on standard C integral conversions of the <> operators and the ternary operator for evaluating the comparison and the result(type). That works mostly great. Except, comparing signed and unsigned values in C leads to oddities and the caller should explicitly take care of that. Add static assertions to check that the compared arguments have the same signedness. --- src/libnm-std-aux/nm-std-aux.h | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/src/libnm-std-aux/nm-std-aux.h b/src/libnm-std-aux/nm-std-aux.h index e65f2c417a..623a7c1e03 100644 --- a/src/libnm-std-aux/nm-std-aux.h +++ b/src/libnm-std-aux/nm-std-aux.h @@ -402,6 +402,9 @@ nm_mult_clamped_u(unsigned a, unsigned b) ({ \ typeof(a) NM_UNIQ_T(A, aq) = (a); \ typeof(b) NM_UNIQ_T(B, bq) = (b); \ + \ + G_STATIC_ASSERT(_NM_INT_SAME_SIGNEDNESS(NM_UNIQ_T(A, aq), NM_UNIQ_T(B, bq))); \ + \ ((NM_UNIQ_T(A, aq) < NM_UNIQ_T(B, bq)) ? NM_UNIQ_T(A, aq) : NM_UNIQ_T(B, bq)); \ }) @@ -410,19 +413,25 @@ nm_mult_clamped_u(unsigned a, unsigned b) ({ \ typeof(a) NM_UNIQ_T(A, aq) = (a); \ typeof(b) NM_UNIQ_T(B, bq) = (b); \ + \ + G_STATIC_ASSERT(_NM_INT_SAME_SIGNEDNESS(NM_UNIQ_T(A, aq), NM_UNIQ_T(B, bq))); \ + \ ((NM_UNIQ_T(A, aq) > NM_UNIQ_T(B, bq)) ? NM_UNIQ_T(A, aq) : NM_UNIQ_T(B, bq)); \ }) #define NM_CLAMP(x, low, high) __NM_CLAMP(NM_UNIQ, x, NM_UNIQ, low, NM_UNIQ, high) -#define __NM_CLAMP(xq, x, lowq, low, highq, high) \ - ({ \ - typeof(x) NM_UNIQ_T(X, xq) = (x); \ - typeof(low) NM_UNIQ_T(LOW, lowq) = (low); \ - typeof(high) NM_UNIQ_T(HIGH, highq) = (high); \ - \ - ((NM_UNIQ_T(X, xq) > NM_UNIQ_T(HIGH, highq)) ? NM_UNIQ_T(HIGH, highq) \ - : (NM_UNIQ_T(X, xq) < NM_UNIQ_T(LOW, lowq)) ? NM_UNIQ_T(LOW, lowq) \ - : NM_UNIQ_T(X, xq)); \ +#define __NM_CLAMP(xq, x, lowq, low, highq, high) \ + ({ \ + typeof(x) NM_UNIQ_T(X, xq) = (x); \ + typeof(low) NM_UNIQ_T(LOW, lowq) = (low); \ + typeof(high) NM_UNIQ_T(HIGH, highq) = (high); \ + \ + G_STATIC_ASSERT(_NM_INT_SAME_SIGNEDNESS(NM_UNIQ_T(X, xq), NM_UNIQ_T(LOW, lowq))); \ + G_STATIC_ASSERT(_NM_INT_SAME_SIGNEDNESS(NM_UNIQ_T(X, xq), NM_UNIQ_T(HIGH, highq))); \ + \ + ((NM_UNIQ_T(X, xq) > NM_UNIQ_T(HIGH, highq)) ? NM_UNIQ_T(HIGH, highq) \ + : (NM_UNIQ_T(X, xq) < NM_UNIQ_T(LOW, lowq)) ? NM_UNIQ_T(LOW, lowq) \ + : NM_UNIQ_T(X, xq)); \ }) #define NM_MAX_WITH_CMP(cmp, a, b) \ From 5fe6b63a62dfd4c3454b17aa80a2bfb4765f8a27 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 31 Jul 2023 11:48:40 +0200 Subject: [PATCH 4/4] std-aux: add static-asserts about signedness for NM_CMP_DIRECT() --- src/libnm-std-aux/nm-std-aux.h | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/libnm-std-aux/nm-std-aux.h b/src/libnm-std-aux/nm-std-aux.h index 623a7c1e03..46d8f2025e 100644 --- a/src/libnm-std-aux/nm-std-aux.h +++ b/src/libnm-std-aux/nm-std-aux.h @@ -1302,13 +1302,15 @@ nm_ptr_to_uintptr(const void *p) /*****************************************************************************/ -#define NM_CMP_DIRECT(a, b) \ - do { \ - typeof(a) _a = (a); \ - typeof(b) _b = (b); \ - \ - if (_a != _b) \ - return (_a < _b) ? -1 : 1; \ +#define NM_CMP_DIRECT(a, b) \ + do { \ + typeof(a) _a = (a); \ + typeof(b) _b = (b); \ + \ + G_STATIC_ASSERT(_NM_INT_SAME_SIGNEDNESS(_a, _b)); \ + \ + if (_a != _b) \ + return (_a < _b) ? -1 : 1; \ } while (0) #define NM_CMP_DIRECT_UNSAFE(a, b) \