From 09c984c6f76be170c00982fab806f85f2631ad2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 18 Nov 2018 10:46:30 +0100 Subject: [PATCH 1/4] test-util: modernize --- src/test/test-util.c | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/src/test/test-util.c b/src/test/test-util.c index 21d58dd86b..80856e6388 100644 --- a/src/test/test-util.c +++ b/src/test/test-util.c @@ -13,11 +13,14 @@ #include "raw-clone.h" #include "rm-rf.h" #include "string-util.h" +#include "tests.h" #include "util.h" static void test_align_power2(void) { unsigned long i, p2; + log_info("/* %s */", __func__); + assert_se(ALIGN_POWER2(0) == 0); assert_se(ALIGN_POWER2(1) == 1); assert_se(ALIGN_POWER2(2) == 2); @@ -60,6 +63,8 @@ static void test_max(void) { void *p = (void *)str; void *q = (void *)&str[16]; + log_info("/* %s */", __func__); + assert_cc(sizeof(val1.b) == sizeof(int) * 100); /* CONST_MAX returns (void) instead of a value if the passed arguments @@ -135,6 +140,8 @@ static void test_container_of(void) { uint32_t v2; } _packed_ myval = { }; + log_info("/* %s */", __func__); + assert_cc(sizeof(myval) == 17); assert_se(container_of(&myval.v1, struct mytype, v1) == &myval); assert_se(container_of(&myval.v2, struct mytype, v2) == &myval); @@ -150,6 +157,8 @@ static void test_container_of(void) { static void test_div_round_up(void) { int div; + log_info("/* %s */", __func__); + /* basic tests */ assert_se(DIV_ROUND_UP(0, 8) == 0); assert_se(DIV_ROUND_UP(1, 8) == 1); @@ -181,6 +190,8 @@ static void test_div_round_up(void) { } static void test_u64log2(void) { + log_info("/* %s */", __func__); + assert_se(u64log2(0) == 0); assert_se(u64log2(8) == 3); assert_se(u64log2(9) == 3); @@ -191,6 +202,8 @@ static void test_u64log2(void) { } static void test_protect_errno(void) { + log_info("/* %s */", __func__); + errno = 12; { PROTECT_ERRNO; @@ -200,6 +213,8 @@ static void test_protect_errno(void) { } static void test_in_set(void) { + log_info("/* %s */", __func__); + assert_se(IN_SET(1, 1)); assert_se(IN_SET(1, 1, 2, 3, 4)); assert_se(IN_SET(2, 1, 2, 3, 4)); @@ -210,6 +225,8 @@ static void test_in_set(void) { } static void test_log2i(void) { + log_info("/* %s */", __func__); + assert_se(log2i(1) == 0); assert_se(log2i(2) == 1); assert_se(log2i(3) == 1); @@ -223,6 +240,8 @@ static void test_log2i(void) { static void test_raw_clone(void) { pid_t parent, pid, pid2; + log_info("/* %s */", __func__); + parent = getpid(); log_info("before clone: getpid()→"PID_FMT, parent); assert_se(raw_getpid() == parent); @@ -253,6 +272,8 @@ static void test_physical_memory(void) { uint64_t p; char buf[FORMAT_BYTES_MAX]; + log_info("/* %s */", __func__); + p = physical_memory(); assert_se(p > 0); assert_se(p < UINT64_MAX); @@ -264,6 +285,8 @@ static void test_physical_memory(void) { static void test_physical_memory_scale(void) { uint64_t p; + log_info("/* %s */", __func__); + p = physical_memory(); assert_se(physical_memory_scale(0, 100) == 0); @@ -298,6 +321,8 @@ static void test_physical_memory_scale(void) { static void test_system_tasks_max(void) { uint64_t t; + log_info("/* %s */", __func__); + t = system_tasks_max(); assert_se(t > 0); assert_se(t < UINT64_MAX); @@ -308,6 +333,8 @@ static void test_system_tasks_max(void) { static void test_system_tasks_max_scale(void) { uint64_t t; + log_info("/* %s */", __func__); + t = system_tasks_max(); assert_se(system_tasks_max_scale(0, 100) == 0); @@ -333,8 +360,7 @@ static void test_system_tasks_max_scale(void) { } int main(int argc, char *argv[]) { - log_parse_environment(); - log_open(); + test_setup_logging(LOG_INFO); test_align_power2(); test_max(); From 7f6bfc561fea20251eb42b9c2fb17348172ccbbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 18 Nov 2018 10:42:39 +0100 Subject: [PATCH 2/4] basic/util: import memeqzero from casync --- src/basic/util.c | 22 ++++++++++++++++++++++ src/basic/util.h | 4 ++++ src/test/test-util.c | 15 +++++++++++++++ 3 files changed, 41 insertions(+) diff --git a/src/basic/util.c b/src/basic/util.c index cd75529cfe..00052c5e1d 100644 --- a/src/basic/util.c +++ b/src/basic/util.c @@ -165,6 +165,28 @@ void *xbsearch_r(const void *key, const void *base, size_t nmemb, size_t size, return NULL; } +bool memeqzero(const void *data, size_t length) { + /* Does the buffer consist entirely of NULs? + * Copied from https://github.com/systemd/casync/, copied in turn from + * https://github.com/rustyrussell/ccan/blob/master/ccan/mem/mem.c#L92, + * which is licensed CC-0. + */ + + const uint8_t *p = data; + size_t i; + + /* Check first 16 bytes manually */ + for (i = 0; i < 16; i++, length--) { + if (length == 0) + return true; + if (p[i]) + return false; + } + + /* Now we know first 16 bytes are NUL, memcmp with self. */ + return memcmp(data, p + i, length) == 0; +} + int on_ac_power(void) { bool found_offline = false, found_online = false; _cleanup_closedir_ DIR *d = NULL; diff --git a/src/basic/util.h b/src/basic/util.h index 8cba4ed726..fd180cb787 100644 --- a/src/basic/util.h +++ b/src/basic/util.h @@ -159,6 +159,10 @@ int on_ac_power(void); #define zero(x) (memzero(&(x), sizeof(x))) +bool memeqzero(const void *data, size_t length); + +#define eqzero(x) memeqzero(x, sizeof(x)) + static inline void *mempset(void *s, int c, size_t n) { memset(s, c, n); return (uint8_t*)s + n; diff --git a/src/test/test-util.c b/src/test/test-util.c index 80856e6388..eeee09a7d2 100644 --- a/src/test/test-util.c +++ b/src/test/test-util.c @@ -237,6 +237,20 @@ static void test_log2i(void) { assert_se(log2i(INT_MAX) == sizeof(int)*8-2); } +static void test_eqzero(void) { + const uint32_t zeros[] = {0, 0, 0}; + const uint32_t ones[] = {1, 1}; + const uint32_t mixed[] = {0, 1, 0, 0, 0}; + const uint8_t longer[] = {[55] = 255}; + + log_info("/* %s */", __func__); + + assert_se(eqzero(zeros)); + assert_se(!eqzero(ones)); + assert_se(!eqzero(mixed)); + assert_se(!eqzero(longer)); +} + static void test_raw_clone(void) { pid_t parent, pid, pid2; @@ -370,6 +384,7 @@ int main(int argc, char *argv[]) { test_protect_errno(); test_in_set(); test_log2i(); + test_eqzero(); test_raw_clone(); test_physical_memory(); test_physical_memory_scale(); From 5dd101186bcb22ccf32d59f8ad4520e6c6006ed6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 18 Nov 2018 10:12:14 +0100 Subject: [PATCH 3/4] udev: use an "inline" array instead of allocating for advertise mode The code is a bit shorter and we don't allocate the uint32_t[127] array. --- src/udev/net/ethtool-util.c | 30 ++++++++++-------------------- src/udev/net/link-config.c | 3 +-- src/udev/net/link-config.h | 2 +- 3 files changed, 12 insertions(+), 23 deletions(-) diff --git a/src/udev/net/ethtool-util.c b/src/udev/net/ethtool-util.c index 05721c71f2..4cf6f269e3 100644 --- a/src/udev/net/ethtool-util.c +++ b/src/udev/net/ethtool-util.c @@ -110,6 +110,8 @@ static const char* const ethtool_link_mode_bit_table[_ETHTOOL_LINK_MODE_MAX] = { [ETHTOOL_LINK_MODE_FEC_RS_BIT] = "fec-rs", [ETHTOOL_LINK_MODE_FEC_BASER_BIT] = "fec-baser", }; +/* Make sure the array is large enough to fit all bits */ +assert_cc((ELEMENTSOF(ethtool_link_mode_bit_table)-1) / 32 < ELEMENTSOF(((struct link_config){}).advertise)); DEFINE_STRING_TABLE_LOOKUP(ethtool_link_mode_bit, enum ethtool_link_mode_bit_indices); @@ -612,8 +614,11 @@ int ethtool_set_glinksettings(int *fd, const char *ifname, struct link_config *l u->base.autoneg = link->autonegotiation; - if (link->advertise) - memcpy(&u->link_modes.advertising, link->advertise, ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NBYTES); + if (!eqzero(link->advertise)) { + memcpy(&u->link_modes.advertising, link->advertise, sizeof(link->advertise)); + memzero((uint8_t*) &u->link_modes.advertising + sizeof(link->advertise), + ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NBYTES - sizeof(link->advertise)); + } if (u->base.cmd == ETHTOOL_GLINKSETTINGS) r = set_slinksettings(*fd, &ifr, u); @@ -737,8 +742,6 @@ int config_parse_advertise(const char *unit, const char *rvalue, void *data, void *userdata) { - uint32_t a[ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NU32] = {}; - enum ethtool_link_mode_bit_indices mode; link_config *config = data; const char *p; int r; @@ -751,12 +754,13 @@ int config_parse_advertise(const char *unit, if (isempty(rvalue)) { /* Empty string resets the value. */ - config->advertise = mfree(config->advertise); + zero(config->advertise); return 0; } for (p = rvalue;;) { _cleanup_free_ char *w = NULL; + enum ethtool_link_mode_bit_indices mode; r = extract_first_word(&p, &w, NULL, 0); if (r == -ENOMEM) @@ -774,21 +778,7 @@ int config_parse_advertise(const char *unit, continue; } - a[mode / 32] |= 1UL << (mode % 32); - } - - if (!config->advertise) { - config->advertise = new(uint32_t, ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NU32); - if (!config->advertise) - return log_oom(); - - memcpy(config->advertise, a, ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NBYTES); - - } else { - unsigned i; - - for (i = 0; i < ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NU32; i++) - config->advertise[i] |= a[i]; + config->advertise[mode / 32] |= 1UL << (mode % 32); } return 0; diff --git a/src/udev/net/link-config.c b/src/udev/net/link-config.c index 161e116d72..becaffe0d6 100644 --- a/src/udev/net/link-config.c +++ b/src/udev/net/link-config.c @@ -69,7 +69,6 @@ static void link_config_free(link_config *link) { free(link->name_policy); free(link->name); free(link->alias); - free(link->advertise); free(link); } @@ -373,7 +372,7 @@ int link_config_apply(link_config_ctx *ctx, link_config *config, if (config->port != _NET_DEV_PORT_INVALID) log_warning_errno(r, "Could not set port (%s) of %s: %m", port_to_string(config->port), old_name); - if (config->advertise) + if (!eqzero(config->advertise)) log_warning_errno(r, "Could not set advertise mode: %m"); /* TODO: include modes in the log message. */ if (config->speed) { diff --git a/src/udev/net/link-config.h b/src/udev/net/link-config.h index f2f0843555..8204959034 100644 --- a/src/udev/net/link-config.h +++ b/src/udev/net/link-config.h @@ -55,7 +55,7 @@ struct link_config { size_t speed; Duplex duplex; int autonegotiation; - uint32_t *advertise; + uint32_t advertise[2]; WakeOnLan wol; NetDevPort port; int features[_NET_DEV_FEAT_MAX]; From 64d9f7568bf8d3c500a9da5c93a2aa88084b901f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 18 Nov 2018 10:13:32 +0100 Subject: [PATCH 4/4] basic/missing: drop _ETHTOOL_LINK_MODE_MAX It was only used in one place, where we don't actually need it, and it is too easy to forget to update it when adding new items to the table. Let's just drop it. --- src/basic/missing.h | 7 ------- src/udev/net/ethtool-util.c | 2 +- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/src/basic/missing.h b/src/basic/missing.h index 85af7acdfe..45788af9e7 100644 --- a/src/basic/missing.h +++ b/src/basic/missing.h @@ -1577,11 +1577,4 @@ enum ethtool_link_mode_bit_indices { #endif #endif -/* Strictly speaking, this is not a missing field. Just for our convention. */ -#if !HAVE_ETHTOOL_LINK_MODE_FEC_NONE_BIT -#define _ETHTOOL_LINK_MODE_MAX 52 -#else -#define _ETHTOOL_LINK_MODE_MAX (__ETHTOOL_LINK_MODE_LAST + 1) -#endif - #include "missing_syscall.h" diff --git a/src/udev/net/ethtool-util.c b/src/udev/net/ethtool-util.c index 4cf6f269e3..177b273e48 100644 --- a/src/udev/net/ethtool-util.c +++ b/src/udev/net/ethtool-util.c @@ -56,7 +56,7 @@ static const char* const netdev_feature_table[_NET_DEV_FEAT_MAX] = { [NET_DEV_FEAT_TSO6] = "tx-tcp6-segmentation", }; -static const char* const ethtool_link_mode_bit_table[_ETHTOOL_LINK_MODE_MAX] = { +static const char* const ethtool_link_mode_bit_table[] = { [ETHTOOL_LINK_MODE_10baseT_Half_BIT] = "10baset-half", [ETHTOOL_LINK_MODE_10baseT_Full_BIT] = "10baset-full", [ETHTOOL_LINK_MODE_100baseT_Half_BIT] = "100baset-half",