From c569d529ec6a10a67527d5678c1d4acf011c6506 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 24 Sep 2019 15:21:47 +0200 Subject: [PATCH 1/4] test-mountpoint-util: improve output a bit --- src/test/test-mountpoint-util.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/test/test-mountpoint-util.c b/src/test/test-mountpoint-util.c index 8c5aac8eb7..499873f0ce 100644 --- a/src/test/test-mountpoint-util.c +++ b/src/test/test-mountpoint-util.c @@ -81,10 +81,11 @@ static void test_mnt_id(void) { continue; } - log_debug("mnt ids of %s are %i, %i\n", p, mnt_id, mnt_id2); - - if (mnt_id == mnt_id2) + if (mnt_id == mnt_id2) { + log_debug("mnt ids of %s is %i\n", p, mnt_id); continue; + } else + log_debug("mnt ids of %s are %i, %i\n", p, mnt_id, mnt_id2); /* The ids don't match? If so, then there are two mounts on the same path, let's check if * that's really the case */ From 5a2904a2f7661526cfbdb19d8d425dad11afc03c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 25 Sep 2019 11:39:10 +0200 Subject: [PATCH 2/4] test-arphdr-list: modernize and provide some useful output --- src/test/test-arphrd-list.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/test/test-arphrd-list.c b/src/test/test-arphrd-list.c index 02bb25b695..e5c50f37dc 100644 --- a/src/test/test-arphrd-list.c +++ b/src/test/test-arphrd-list.c @@ -5,6 +5,7 @@ #include "macro.h" #include "string-util.h" +#include "tests.h" _unused_ \ static const struct arphrd_name* lookup_arphrd(register const char *str, register GPERF_LEN_TYPE len); @@ -14,15 +15,15 @@ static const struct arphrd_name* lookup_arphrd(register const char *str, registe #include "arphrd-to-name.h" int main(int argc, const char *argv[]) { + test_setup_logging(LOG_INFO); - unsigned i; - - for (i = 1; i < ELEMENTSOF(arphrd_names); i++) { + for (unsigned i = 1; i < ELEMENTSOF(arphrd_names); i++) if (arphrd_names[i]) { + log_info("%i: %s", i, arphrd_to_name(i)); + assert_se(streq(arphrd_to_name(i), arphrd_names[i])); assert_se(arphrd_from_name(arphrd_names[i]) == (int) i); } - } assert_se(arphrd_to_name(arphrd_max()) == NULL); assert_se(arphrd_to_name(0) == NULL); From 7e833f301eeca5114be3315ec4f4385cf34977af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 25 Sep 2019 12:06:05 +0200 Subject: [PATCH 3/4] basic: massively reduce the size of arphdr lookup functions Our biggest object in libsystemd was a table full of zeros, for the arphdr names. Let's use a switch (which gcc nicely optimizes for us), instead a table with a gap between 826 and 65534: $ ls -l build{,2}/src/basic/a6ba3eb@@basic@sta/arphrd-list.c.o -rw-rw-r--. 1 zbyszek zbyszek 540232 Sep 22 00:29 build/src/basic/a6ba3eb\@\@basic\@sta/arphrd-list.c.o -rw-rw-r--. 1 zbyszek zbyszek 20512 Sep 25 11:56 build2/src/basic/a6ba3eb\@\@basic\@sta/arphrd-list.c.o $ ls -l build{,2}/src/shared/libsystemd-shared-243.so -rwxrwxr-x. 1 zbyszek zbyszek 6774368 Sep 22 00:29 build/src/shared/libsystemd-shared-243.so -rwxrwxr-x. 1 zbyszek zbyszek 6254808 Sep 25 12:16 build2/src/shared/libsystemd-shared-243.so No functional change. --- src/basic/arphrd-list.c | 15 --------------- src/basic/arphrd-list.h | 2 -- src/basic/arphrd-to-name.awk | 11 +++++++---- src/test/test-arphrd-list.c | 22 +++++++++------------- 4 files changed, 16 insertions(+), 34 deletions(-) diff --git a/src/basic/arphrd-list.c b/src/basic/arphrd-list.c index b9a9cb7ed4..7e5570ab9f 100644 --- a/src/basic/arphrd-list.c +++ b/src/basic/arphrd-list.c @@ -12,17 +12,6 @@ static const struct arphrd_name* lookup_arphrd(register const char *str, registe #include "arphrd-from-name.h" #include "arphrd-to-name.h" -const char *arphrd_to_name(int id) { - - if (id <= 0) - return NULL; - - if ((size_t) id >= ELEMENTSOF(arphrd_names)) - return NULL; - - return arphrd_names[id]; -} - int arphrd_from_name(const char *name) { const struct arphrd_name *sc; @@ -34,7 +23,3 @@ int arphrd_from_name(const char *name) { return sc->id; } - -int arphrd_max(void) { - return ELEMENTSOF(arphrd_names); -} diff --git a/src/basic/arphrd-list.h b/src/basic/arphrd-list.h index 5dcfe5e12d..aae56bc88c 100644 --- a/src/basic/arphrd-list.h +++ b/src/basic/arphrd-list.h @@ -3,5 +3,3 @@ const char *arphrd_to_name(int id); int arphrd_from_name(const char *name); - -int arphrd_max(void); diff --git a/src/basic/arphrd-to-name.awk b/src/basic/arphrd-to-name.awk index 5a35673e2c..4600dbf28e 100644 --- a/src/basic/arphrd-to-name.awk +++ b/src/basic/arphrd-to-name.awk @@ -1,9 +1,12 @@ BEGIN{ - print "static const char* const arphrd_names[] = { " + print "const char *arphrd_to_name(int id) {" + print " switch(id) {" } -!/CISCO/ { - printf " [ARPHRD_%s] = \"%s\",\n", $1, $1 +!/CISCO|NETROM/ { + printf " case ARPHRD_%s: return \"%s\";\n", $1, $1 } END{ - print "};" + print " default: return NULL;" + print " }" + print "}" } diff --git a/src/test/test-arphrd-list.c b/src/test/test-arphrd-list.c index e5c50f37dc..3010ca6b45 100644 --- a/src/test/test-arphrd-list.c +++ b/src/test/test-arphrd-list.c @@ -1,31 +1,27 @@ /* SPDX-License-Identifier: LGPL-2.1+ */ #include -#include -#include "macro.h" #include "string-util.h" #include "tests.h" -_unused_ \ -static const struct arphrd_name* lookup_arphrd(register const char *str, register GPERF_LEN_TYPE len); - -#include "arphrd-from-name.h" #include "arphrd-list.h" -#include "arphrd-to-name.h" int main(int argc, const char *argv[]) { test_setup_logging(LOG_INFO); - for (unsigned i = 1; i < ELEMENTSOF(arphrd_names); i++) - if (arphrd_names[i]) { - log_info("%i: %s", i, arphrd_to_name(i)); + for (int i = 0; i <= ARPHRD_VOID + 1; i++) { + const char *name; - assert_se(streq(arphrd_to_name(i), arphrd_names[i])); - assert_se(arphrd_from_name(arphrd_names[i]) == (int) i); + name = arphrd_to_name(i); + if (name) { + log_info("%i: %s", i, name); + + assert_se(arphrd_from_name(name) == i); } + } - assert_se(arphrd_to_name(arphrd_max()) == NULL); + assert_se(arphrd_to_name(ARPHRD_VOID + 1) == NULL); assert_se(arphrd_to_name(0) == NULL); assert_se(arphrd_from_name("huddlduddl") == -EINVAL); assert_se(arphrd_from_name("") == -EINVAL); From 35bdba21151798c8204661726c21ae4a49e4662b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 25 Sep 2019 12:14:07 +0200 Subject: [PATCH 4/4] basic/arphrd: stop discriminating against NETROM and CISCO ARPHRD_NETROM was excluded, most likely just because it is protocol No. 0, and ARPHRD_CISCO was reported under its alias name "HDLC". Let's just allow defined aliases under the main name. --- src/basic/arphrd-to-name.awk | 2 +- src/test/test-arphrd-list.c | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/basic/arphrd-to-name.awk b/src/basic/arphrd-to-name.awk index 4600dbf28e..db1c739abb 100644 --- a/src/basic/arphrd-to-name.awk +++ b/src/basic/arphrd-to-name.awk @@ -2,7 +2,7 @@ BEGIN{ print "const char *arphrd_to_name(int id) {" print " switch(id) {" } -!/CISCO|NETROM/ { +!/^HDLC$/ { printf " case ARPHRD_%s: return \"%s\";\n", $1, $1 } END{ diff --git a/src/test/test-arphrd-list.c b/src/test/test-arphrd-list.c index 3010ca6b45..d7352b54e0 100644 --- a/src/test/test-arphrd-list.c +++ b/src/test/test-arphrd-list.c @@ -22,7 +22,6 @@ int main(int argc, const char *argv[]) { } assert_se(arphrd_to_name(ARPHRD_VOID + 1) == NULL); - assert_se(arphrd_to_name(0) == NULL); assert_se(arphrd_from_name("huddlduddl") == -EINVAL); assert_se(arphrd_from_name("") == -EINVAL);