From: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Date: Mon, 19 Jul 2021 17:40:41 +0000 (+0200)
Subject: networkd: fix and simplify format_lifetime()
X-Git-Tag: v250-rc1~939^2~1
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=a35f3128e606082c35504a367109105ad463157f;p=thirdparty%2Fsystemd.git

networkd: fix and simplify format_lifetime()

We would copy "forever" into the buffer. This is a fairly common case, so let's
do a microoptimization and return a static string. (All callers use the return
pointer, so this works just as well.)

The prefix "for " was not displayed, because the pointer to the part of the
buffer after "for " was returned. (Maybe it's just me, but I find strpcpy()
and associated functions really hard to use… I always have to look up what the
do exactly and what the return value is.)

A simple test is added.
---

diff --git a/src/network/meson.build b/src/network/meson.build
index 4e137d7b9e2..2ea12c8d030 100644
--- a/src/network/meson.build
+++ b/src/network/meson.build
@@ -268,6 +268,12 @@ fuzzers += [
 ]
 
 tests += [
+        [['src/network/test-networkd-address.c'],
+         [libnetworkd_core,
+          libsystemd_network],
+         [],
+         network_includes],
+
         [['src/network/test-networkd-conf.c'],
          [libnetworkd_core,
           libsystemd_network],
diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c
index 2d3f8ece2d2..2f1dcfc62bd 100644
--- a/src/network/networkd-address.c
+++ b/src/network/networkd-address.c
@@ -621,19 +621,17 @@ int manager_has_address(Manager *manager, int family, const union in_addr_union
         return false;
 }
 
-char *format_lifetime(char *buf, size_t l, uint32_t lifetime) {
-        char *p = buf;
-
+const char* format_lifetime(char *buf, size_t l, uint32_t lifetime) {
         assert(buf);
-        assert(l > 0);
+        assert(l > 4);
 
-        if (lifetime == CACHE_INFO_INFINITY_LIFE_TIME) {
-                strscpy(buf, l, "forever");
-                return buf;
-        }
+        if (lifetime == CACHE_INFO_INFINITY_LIFE_TIME)
+                return "forever";
 
-        l -= strpcpy(&p, l, "for ");
-        return format_timespan(p, l, lifetime * USEC_PER_SEC, USEC_PER_SEC);
+        sprintf(buf, "for ");
+        /* format_timespan() never fails */
+        assert_se(format_timespan(buf + 4, l - 4, lifetime * USEC_PER_SEC, USEC_PER_SEC));
+        return buf;
 }
 
 static void log_address_debug(const Address *address, const char *str, const Link *link) {
diff --git a/src/network/networkd-address.h b/src/network/networkd-address.h
index 1569b588a02..811940c1261 100644
--- a/src/network/networkd-address.h
+++ b/src/network/networkd-address.h
@@ -50,7 +50,7 @@ typedef struct Address {
         address_ready_callback_t callback;
 } Address;
 
-char *format_lifetime(char *buf, size_t l, uint32_t lifetime) _warn_unused_result_;
+const char* format_lifetime(char *buf, size_t l, uint32_t lifetime) _warn_unused_result_;
 /* Note: the lifetime of the compound literal is the immediately surrounding block,
  * see C11 §6.5.2.5, and
  * https://stackoverflow.com/questions/34880638/compound-literal-lifetime-and-if-blocks */
@@ -58,7 +58,7 @@ char *format_lifetime(char *buf, size_t l, uint32_t lifetime) _warn_unused_resul
         format_lifetime((char[FORMAT_TIMESPAN_MAX+STRLEN("for ")]){}, FORMAT_TIMESPAN_MAX+STRLEN("for "), lifetime)
 
 int address_new(Address **ret);
-Address *address_free(Address *address);
+Address* address_free(Address *address);
 int address_get(Link *link, const Address *in, Address **ret);
 int address_configure_handler_internal(sd_netlink *rtnl, sd_netlink_message *m, Link *link, const char *error_msg);
 int address_remove(const Address *address, Link *link);
diff --git a/src/network/test-networkd-address.c b/src/network/test-networkd-address.c
new file mode 100644
index 00000000000..7c1d65a433c
--- /dev/null
+++ b/src/network/test-networkd-address.c
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+
+#include "networkd-address.h"
+#include "tests.h"
+
+static void test_FORMAT_LIFETIME_one(uint32_t lifetime, const char *expected) {
+        const char *t = FORMAT_LIFETIME(lifetime);
+
+        log_debug("%"PRIu32 " → \"%s\" (expected \"%s\")", lifetime, t, expected);
+        assert_se(streq(t, expected));
+}
+
+static void test_FORMAT_LIFETIME(void) {
+        log_info("/* %s */", __func__);
+
+        test_FORMAT_LIFETIME_one(0, "for 0");
+        test_FORMAT_LIFETIME_one(1, "for 1s");
+        test_FORMAT_LIFETIME_one(3 * (USEC_PER_WEEK/USEC_PER_SEC), "for 3w");
+        test_FORMAT_LIFETIME_one(CACHE_INFO_INFINITY_LIFE_TIME, "forever");
+}
+
+int main(int argc, char *argv[]) {
+        test_setup_logging(LOG_INFO);
+
+        test_FORMAT_LIFETIME();
+
+        return 0;
+}