From c36354b26c757e526e9f3d8c5bc78aa36f095f61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= 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); }