From c36354b26c757e526e9f3d8c5bc78aa36f095f61 Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>

Date: Thu, 20 Jan 2022 15:47:22 +0100

Subject: [PATCH] shared/specifier: treat NULL the same as ""

We would busily allocate an empty string to concatenate all of it's

zero characters to the output. Let's make things a bit simpler by letting

the specifier functions return NULL to mean "nothing to append".

(cherry picked from commit 01c69460811f64e416c3e4a545ef84787bb6700b)

Related: #2082131

---

 src/shared/specifier.c    | 56 ++++++++++++++-------------------------

 src/test/test-specifier.c |  9 ++++---

 2 files changed, 25 insertions(+), 40 deletions(-)

diff --git a/src/shared/specifier.c b/src/shared/specifier.c

index 1fd76b1d15..f8ab98541f 100644

--- a/src/shared/specifier.c

+++ b/src/shared/specifier.c

@@ -35,7 +35,6 @@

 int specifier_printf(const char *text, size_t max_length, const Specifier table[], const char *root, const void *userdata, char **ret) {

         _cleanup_free_ char *result = NULL;

         bool percent = false;

-        const char *f;

         size_t l;

         char *t;

         int r;

@@ -48,8 +47,10 @@ int specifier_printf(const char *text, size_t max_length, const Specifier table[

                 return -ENOMEM;

         t = result;

-        for (f = text; *f != '\0'; f++, l--) {

+        for (const char *f = text; *f != '\0'; f++, l--) {

                 if (percent) {

+                        percent = false;

+

                         if (*f == '%')

                                 *(t++) = '%';

                         else {

@@ -66,6 +67,8 @@ int specifier_printf(const char *text, size_t max_length, const Specifier table[

                                         r = i->lookup(i->specifier, i->data, root, userdata, &w);

                                         if (r < 0)

                                                 return r;

+                                        if (isempty(w))

+                                                continue;

                                         j = t - result;

                                         k = strlen(w);

@@ -82,8 +85,6 @@ int specifier_printf(const char *text, size_t max_length, const Specifier table[

                                         *(t++) = *f;

                                 }

                         }

-

-                        percent = false;

                 } else if (*f == '%')

                         percent = true;

                 else

@@ -108,11 +109,13 @@ int specifier_printf(const char *text, size_t max_length, const Specifier table[

 /* Generic handler for simple string replacements */

 int specifier_string(char specifier, const void *data, const char *root, const void *userdata, char **ret) {

-        char *n;

+        char *n = NULL;

-        n = strdup(strempty(data));

-        if (!n)

-                return -ENOMEM;

+        if (!isempty(data)) {

+                n = strdup(data);

+                if (!n)

+                        return -ENOMEM;

+        }

         *ret = n;

         return 0;

@@ -186,10 +189,8 @@ int specifier_short_host_name(char specifier, const void *data, const char *root

 int specifier_kernel_release(char specifier, const void *data, const char *root, const void *userdata, char **ret) {

         struct utsname uts;

         char *n;

-        int r;

-        r = uname(&uts;;

-        if (r < 0)

+        if (uname(&uts) < 0)

                 return -errno;

         n = strdup(uts.release);

@@ -211,47 +212,31 @@ int specifier_architecture(char specifier, const void *data, const char *root, c

         return 0;

 }

-static int specifier_os_release_common(const char *field, const char *root, char **ret) {

-        char *t = NULL;

-        int r;

-

-        r = parse_os_release(root, field, &t);

-        if (r < 0)

-                return r;

-        if (!t) {

-                /* fields in /etc/os-release might quite possibly be missing, even if everything is entirely

-                 * valid otherwise. Let's hence return "" in that case. */

-                t = strdup("");

-                if (!t)

-                        return -ENOMEM;

-        }

-

-        *ret = t;

-        return 0;

-}

+/* Note: fields in /etc/os-release might quite possibly be missing, even if everything is entirely valid

+ * otherwise. We'll return an empty value or NULL in that case from the functions below. */

 int specifier_os_id(char specifier, const void *data, const char *root, const void *userdata, char **ret) {

-        return specifier_os_release_common("ID", root, ret);

+        return parse_os_release(root, "ID", ret);

 }

 int specifier_os_version_id(char specifier, const void *data, const char *root, const void *userdata, char **ret) {

-        return specifier_os_release_common("VERSION_ID", root, ret);

+        return parse_os_release(root, "VERSION_ID", ret);

 }

 int specifier_os_build_id(char specifier, const void *data, const char *root, const void *userdata, char **ret) {

-        return specifier_os_release_common("BUILD_ID", root, ret);

+        return parse_os_release(root, "BUILD_ID", ret);

 }

 int specifier_os_variant_id(char specifier, const void *data, const char *root, const void *userdata, char **ret) {

-        return specifier_os_release_common("VARIANT_ID", root, ret);

+        return parse_os_release(root, "VARIANT_ID", ret);

 }

 int specifier_os_image_id(char specifier, const void *data, const char *root, const void *userdata, char **ret) {

-        return specifier_os_release_common("IMAGE_ID", root, ret);

+        return parse_os_release(root, "IMAGE_ID", ret);

 }

 int specifier_os_image_version(char specifier, const void *data, const char *root, const void *userdata, char **ret) {

-        return specifier_os_release_common("IMAGE_VERSION", root, ret);

+        return parse_os_release(root, "IMAGE_VERSION", ret);

 }

 int specifier_group_name(char specifier, const void *data, const char *root, const void *userdata, char **ret) {

@@ -291,7 +276,6 @@ int specifier_user_name(char specifier, const void *data, const char *root, cons

 }

 int specifier_user_id(char specifier, const void *data, const char *root, const void *userdata, char **ret) {

-

         if (asprintf(ret, UID_FMT, getuid()) < 0)

                 return -ENOMEM;

diff --git a/src/test/test-specifier.c b/src/test/test-specifier.c

index 40957eeb59..dda993ce9d 100644

--- a/src/test/test-specifier.c

+++ b/src/test/test-specifier.c

@@ -56,6 +56,7 @@ TEST(specifier_printf) {

         static const Specifier table[] = {

                 { 'X', specifier_string,         (char*) "AAAA" },

                 { 'Y', specifier_string,         (char*) "BBBB" },

+                { 'e', specifier_string,         NULL           },

                 COMMON_SYSTEM_SPECIFIERS,

                 {}

         };

@@ -63,21 +64,21 @@ TEST(specifier_printf) {

         _cleanup_free_ char *w = NULL;

         int r;

-        r = specifier_printf("xxx a=%X b=%Y yyy", SIZE_MAX, table, NULL, NULL, &w);

+        r = specifier_printf("xxx a=%X b=%Y e=%e yyy", SIZE_MAX, table, NULL, NULL, &w);

         assert_se(r >= 0);

         assert_se(w);

         puts(w);

-        assert_se(streq(w, "xxx a=AAAA b=BBBB yyy"));

+        assert_se(streq(w, "xxx a=AAAA b=BBBB e= yyy"));

         free(w);

-        r = specifier_printf("machine=%m, boot=%b, host=%H, version=%v, arch=%a", SIZE_MAX, table, NULL, NULL, &w);

+        r = specifier_printf("machine=%m, boot=%b, host=%H, version=%v, arch=%a, empty=%e", SIZE_MAX, table, NULL, NULL, &w);

         assert_se(r >= 0);

         assert_se(w);

         puts(w);

         w = mfree(w);

-        specifier_printf("os=%o, os-version=%w, build=%B, variant=%W", SIZE_MAX, table, NULL, NULL, &w);

+        specifier_printf("os=%o, os-version=%w, build=%B, variant=%W, empty=%e%e%e", SIZE_MAX, table, NULL, NULL, &w);

         if (w)

                 puts(w);

 }