Refactor strv_env_replace() into strv_env_replace_consume()

All callers of strv_env_replace() would free the argument on error.
So let's follow the same pattern as with strv_consume (and similar
naming) and unconditionally "use up" the argument.

diff --git a/src/basic/env-file.c b/src/basic/env-file.c
index 99c3e3f4a31f3a7b41bc5876338c0d8a35b729a9..11362d8556b88da20cc9718ce4844faf04043202 100644 (file)
--- a/src/basic/env-file.c
+++ b/src/basic/env-file.c
@@ -385,11 +385,9 @@ static int load_env_file_push(
         if (!p)
                 return -ENOMEM;
 
-        r = strv_env_replace(m, p);
-        if (r < 0) {
-                free(p);
+        r = strv_env_replace_consume(m, p);
+        if (r < 0)
                 return r;
-        }
 
         if (n_pushed)
                 (*n_pushed)++;

diff --git a/src/basic/env-util.c b/src/basic/env-util.c
index b1313dbfff8d3c09be14fa741658f669366c9d5e..25c4b7c5a4198b49ba82ebc2687ea2b0ebdf7a77 100644 (file)
--- a/src/basic/env-util.c
+++ b/src/basic/env-util.c
@@ -363,7 +363,7 @@ char **strv_env_unset_many(char **l, ...) {
         return l;
 }
 
-int strv_env_replace(char ***l, char *p) {
+int strv_env_replace_consume(char ***l, char *p) {
         const char *t, *name;
         char **f;
         int r;
@@ -371,11 +371,15 @@ int strv_env_replace(char ***l, char *p) {
         assert(p);
 
         /* Replace first occurrence of the env var or add a new one in the string list. Drop other
-         * occurrences. Edits in-place. Does not copy p. p must be a valid key=value assignment. */
+         * occurrences. Edits in-place. Does not copy p and CONSUMES p EVEN ON FAILURE.
+         *
+         * p must be a valid key=value assignment. */
 
         t = strchr(p, '=');
-        if (!t)
+        if (!t) {
+                free(p);
                 return -EINVAL;
+        }
 
         name = strndupa(p, t - p);
 
@@ -387,7 +391,7 @@ int strv_env_replace(char ***l, char *p) {
                 }
 
         /* We didn't find a match, we need to append p or create a new strv */
-        r = strv_push(l, p);
+        r = strv_consume(l, p);
         if (r < 0)
                 return r;
 
@@ -395,24 +399,16 @@ int strv_env_replace(char ***l, char *p) {
 }
 
 int strv_env_replace_strdup(char ***l, const char *assignment) {
-        int r;
+        /* Like strv_env_replace_consume(), but copies the argument. */
 
-        /* Like strv_env_replace(), but copies the argument. */
-
-        _cleanup_free_ char *p = strdup(assignment);
+        char *p = strdup(assignment);
         if (!p)
                 return -ENOMEM;
 
-        r = strv_env_replace(l, p);
-        if (r < 0)
-                return r;
-        TAKE_PTR(p);
-        return r;
+        return strv_env_replace_consume(l, p);
 }
 
 int strv_env_assign(char ***l, const char *key, const char *value) {
-        int r;
-
         if (!env_name_is_valid(key))
                 return -EINVAL;
 
@@ -427,11 +423,7 @@ int strv_env_assign(char ***l, const char *key, const char *value) {
         if (!p)
                 return -ENOMEM;
 
-        r = strv_env_replace(l, p);
-        if (r < 0)
-                return r;
-        TAKE_PTR(p);
-        return r;
+        return strv_env_replace_consume(l, p);
 }
 
 char *strv_env_get_n(char **l, const char *name, size_t k, unsigned flags) {

diff --git a/src/basic/env-util.h b/src/basic/env-util.h
index c7b72b4b471f6fecea6ec0fd8e5f2dafd46b9972..347ea33e666f2384ff294663861f126e680a778b 100644 (file)
--- a/src/basic/env-util.h
+++ b/src/basic/env-util.h
@@ -44,7 +44,7 @@ char **strv_env_delete(char **x, size_t n_lists, ...); /* New copy */
 
 char **strv_env_unset(char **l, const char *p); /* In place ... */
 char **strv_env_unset_many(char **l, ...) _sentinel_;
-int strv_env_replace(char ***l, char *p); /* In place ... */
+int strv_env_replace_consume(char ***l, char *p); /* In place ... */
 int strv_env_replace_strdup(char ***l, const char *assignment);
 int strv_env_assign(char ***l, const char *key, const char *value);
 

diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c
index d0aeaf1c76b7090fc46e56512de4242bb79d6681..dfab38f77bd688252786153b3d905d579cd87ef1 100644 (file)
--- a/src/core/load-fragment.c
+++ b/src/core/load-fragment.c
@@ -2636,7 +2636,7 @@ int config_parse_environ(
         }
 
         for (const char *p = rvalue;; ) {
-                _cleanup_free_ char *word = NULL, *k = NULL;
+                _cleanup_free_ char *word = NULL, *resolved = NULL;
 
                 r = extract_first_word(&p, &word, NULL, EXTRACT_CUNESCAPE|EXTRACT_UNQUOTE);
                 if (r == 0)
@@ -2650,26 +2650,24 @@ int config_parse_environ(
                 }
 
                 if (u) {
-                        r = unit_full_printf(u, word, &k);
+                        r = unit_full_printf(u, word, &resolved);
                         if (r < 0) {
                                 log_syntax(unit, LOG_WARNING, filename, line, r,
                                            "Failed to resolve unit specifiers in %s, ignoring: %m", word);
                                 continue;
                         }
                 } else
-                        k = TAKE_PTR(word);
+                        resolved = TAKE_PTR(word);
 
-                if (!env_assignment_is_valid(k)) {
+                if (!env_assignment_is_valid(resolved)) {
                         log_syntax(unit, LOG_WARNING, filename, line, 0,
-                                   "Invalid environment assignment, ignoring: %s", k);
+                                   "Invalid environment assignment, ignoring: %s", resolved);
                         continue;
                 }
 
-                r = strv_env_replace(env, k);
+                r = strv_env_replace_consume(env, TAKE_PTR(resolved));
                 if (r < 0)
-                        return log_oom();
-
-                k = NULL;
+                        return log_error_errno(r, "Failed to update environment: %m");
         }
 }
 

diff --git a/src/shared/serialize.c b/src/shared/serialize.c
index 45f57d6a5316ef93a70221b1be46f837880c175d..42fe5db3fafbc4120b2e8fdc85622c5097783a0c 100644 (file)
--- a/src/shared/serialize.c
+++ b/src/shared/serialize.c
@@ -175,7 +175,7 @@ int deserialize_dual_timestamp(const char *value, dual_timestamp *t) {
 }
 
 int deserialize_environment(const char *value, char ***list) {
-        _cleanup_free_ char *unescaped = NULL;
+        char *unescaped;
         int r;
 
         assert(value);
@@ -187,11 +187,9 @@ int deserialize_environment(const char *value, char ***list) {
         if (r < 0)
                 return log_error_errno(r, "Failed to unescape: %m");
 
-        r = strv_env_replace(list, unescaped);
+        r = strv_env_replace_consume(list, unescaped);
         if (r < 0)
                 return log_error_errno(r, "Failed to append environment variable: %m");
-
-        unescaped = NULL; /* now part of 'list' */
         return 0;
 }