From 020003f235ca2e5e7897b4bd1d4a0dba7c463b08 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Fri, 30 Oct 2020 10:13:27 +0100 Subject: [PATCH 1/4] string-util: simplify logic in strjoin_real() The loops over (x, then all varargs, until a NULL is found) can be written much simpler with an ordinary for loop. Just initialize the loop variable to x, test that, and in the increment part, fetch the next va_arg(). That removes a level of indentation, and avoids doing a separate strlen()/stpcpy() call for x. While touching this code anyway, change (size_t)-1 to the more readable SIZE_MAX. --- src/basic/string-util.c | 55 +++++++++++------------------------------ 1 file changed, 15 insertions(+), 40 deletions(-) diff --git a/src/basic/string-util.c b/src/basic/string-util.c index ab725d0dab..c8993000b0 100644 --- a/src/basic/string-util.c +++ b/src/basic/string-util.c @@ -145,57 +145,32 @@ char *strnappend(const char *s, const char *suffix, size_t b) { char *strjoin_real(const char *x, ...) { va_list ap; - size_t l; + size_t l = 0; char *r, *p; va_start(ap, x); + for (const char *t = x; t; t = va_arg(ap, const char *)) { + size_t n; - if (x) { - l = strlen(x); - - for (;;) { - const char *t; - size_t n; - - t = va_arg(ap, const char *); - if (!t) - break; - - n = strlen(t); - if (n > ((size_t) -1) - l) { - va_end(ap); - return NULL; - } - - l += n; + n = strlen(t); + if (n > SIZE_MAX - l) { + va_end(ap); + return NULL; } - } else - l = 0; - + l += n; + } va_end(ap); - r = new(char, l+1); + p = r = new(char, l+1); if (!r) return NULL; - if (x) { - p = stpcpy(r, x); + va_start(ap, x); + for (const char *t = x; t; t = va_arg(ap, const char *)) + p = stpcpy(p, t); + va_end(ap); - va_start(ap, x); - - for (;;) { - const char *t; - - t = va_arg(ap, const char *); - if (!t) - break; - - p = stpcpy(p, t); - } - - va_end(ap); - } else - r[0] = 0; + *p = 0; return r; } From 6ced0770c741170a05057dffbf3ef78e46eafe53 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Fri, 30 Oct 2020 10:18:04 +0100 Subject: [PATCH 2/4] string-util: improve overflow checking The current overflow checking is broken in the corner case of the strings' combined length being exactly SIZE_MAX: After the loop, l would be SIZE_MAX, but we're not testing whether the l+1 expression overflows. Fix it by simply pre-accounting for the final '\0': initialize l to 1 instead of 0. --- src/basic/string-util.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/basic/string-util.c b/src/basic/string-util.c index c8993000b0..12c4ae177a 100644 --- a/src/basic/string-util.c +++ b/src/basic/string-util.c @@ -145,7 +145,7 @@ char *strnappend(const char *s, const char *suffix, size_t b) { char *strjoin_real(const char *x, ...) { va_list ap; - size_t l = 0; + size_t l = 1; char *r, *p; va_start(ap, x); @@ -161,7 +161,7 @@ char *strjoin_real(const char *x, ...) { } va_end(ap); - p = r = new(char, l+1); + p = r = new(char, l); if (!r) return NULL; From bd8e699c27fbc52dd7a2c232722ba59222d3d47d Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Fri, 30 Oct 2020 10:27:55 +0100 Subject: [PATCH 3/4] signal-util: make sigaction_many_ap a little more concise There's no reason to duplicate the stop condition sig < 0, nor the sigaction() call. --- src/basic/signal-util.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/basic/signal-util.c b/src/basic/signal-util.c index cb59f6ca0f..e976205d86 100644 --- a/src/basic/signal-util.c +++ b/src/basic/signal-util.c @@ -49,16 +49,7 @@ static int sigaction_many_ap(const struct sigaction *sa, int sig, va_list ap) { int r = 0; /* negative signal ends the list. 0 signal is skipped. */ - - if (sig < 0) - return 0; - - if (sig > 0) { - if (sigaction(sig, sa, NULL) < 0) - r = -errno; - } - - while ((sig = va_arg(ap, int)) >= 0) { + for (; sig >= 0; sig = va_arg(ap, int)) { if (sig == 0) continue; From 3d2b1fa473691cf9b7cbb407a5c75df4bf8b2ae3 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Fri, 30 Oct 2020 10:54:15 +0100 Subject: [PATCH 4/4] strv.c: simplify strv_new_ap Instead of duplicating the code for x and the varargs, handle them all the same way by using for loops. --- src/basic/strv.c | 42 +++++++++++++----------------------------- 1 file changed, 13 insertions(+), 29 deletions(-) diff --git a/src/basic/strv.c b/src/basic/strv.c index b2b6de388a..7a69107462 100644 --- a/src/basic/strv.c +++ b/src/basic/strv.c @@ -123,7 +123,6 @@ size_t strv_length(char * const *l) { } char **strv_new_ap(const char *x, va_list ap) { - const char *s; _cleanup_strv_free_ char **a = NULL; size_t n = 0, i = 0; va_list aq; @@ -133,43 +132,28 @@ char **strv_new_ap(const char *x, va_list ap) { * STRV_IFNOTNULL() macro to include possibly NULL strings in * the string list. */ - if (x) { - n = x == STRV_IGNORE ? 0 : 1; + va_copy(aq, ap); + for (const char *s = x; s; s = va_arg(aq, const char*)) { + if (s == STRV_IGNORE) + continue; - va_copy(aq, ap); - while ((s = va_arg(aq, const char*))) { - if (s == STRV_IGNORE) - continue; - - n++; - } - - va_end(aq); + n++; } + va_end(aq); a = new(char*, n+1); if (!a) return NULL; - if (x) { - if (x != STRV_IGNORE) { - a[i] = strdup(x); - if (!a[i]) - return NULL; - i++; - } + for (const char *s = x; s; s = va_arg(ap, const char*)) { + if (s == STRV_IGNORE) + continue; - while ((s = va_arg(ap, const char*))) { + a[i] = strdup(s); + if (!a[i]) + return NULL; - if (s == STRV_IGNORE) - continue; - - a[i] = strdup(s); - if (!a[i]) - return NULL; - - i++; - } + i++; } a[i] = NULL;