env-util,fileio: immediately replace variables in load_env_file_push()

strv_env_replace was calling env_match(), which in effect allowed multiple
values for the same key to be inserted into the environment block. That's
pointless, because APIs to access variables only return a single value (the
latest entry), so it's better to keep the block clean, i.e. with just a single
entry for each key.

Add a new helper function that simply tests if the part before '=' is equal in
two strings and use that in strv_env_replace.

In load_env_file_push, use strv_env_replace to immediately replace the previous
assignment with a matching name.

Afaict, none of the callers are materially affected by this change, but it
seems like some pointless work was being done, if the same value was set
multiple times. We'd go through parsing and assigning the value for each
entry. With this change, we handle just the last one.

diff --git a/src/basic/env-util.c b/src/basic/env-util.c
index 4645ec781ee83fe1522bef677e28273b66143206..05b90e499e5ea836c06ccb89762448b49e5d8586 100644 (file)
--- a/src/basic/env-util.c
+++ b/src/basic/env-util.c
@@ -274,6 +274,19 @@ _pure_ static bool env_match(const char *t, const char *pattern) {
         return false;
 }
 
+static bool env_entry_has_name(const char *entry, const char *name) {
+        const char *t;
+
+        assert(entry);
+        assert(name);
+
+        t = startswith(entry, name);
+        if (!t)
+                return false;
+
+        return *t == '=';
+}
+
 char **strv_env_delete(char **x, unsigned n_lists, ...) {
         size_t n, i = 0;
         char **k, **r;
@@ -387,18 +400,24 @@ char **strv_env_unset_many(char **l, ...) {
 
 int strv_env_replace(char ***l, char *p) {
         char **f;
+        const char *t, *name;
 
         assert(p);
 
         /* Replace first occurrence of the env var or add a new one in the
          * string list. Drop other occurences. Edits in-place. Does not copy p.
+         * p must be a valid key=value assignment.
          */
 
+        t = strchr(p, '=');
+        assert(t);
+
+        name = strndupa(p, t - p);
+
         for (f = *l; f && *f; f++)
-                if (env_match(*f, p)) {
-                        free(*f);
-                        *f = p;
-                        strv_env_unset(f + 1, p);
+                if (env_entry_has_name(*f, name)) {
+                        free_and_replace(*f, p);
+                        strv_env_unset(f + 1, *f);
                         return 0;
                 }
 
@@ -676,5 +695,8 @@ int deserialize_environment(char ***environment, const char *line) {
         if (r < 0)
                 return r;
 
+        if (!env_assignment_is_valid(uce))
+                return -EINVAL;
+
         return strv_env_replace(environment, uce);
 }

diff --git a/src/basic/fileio.c b/src/basic/fileio.c
index fb41431ec905c03a949f58530b3277cbfd9ea623..a1e4978125dd26f037e3e03cc69fac582cbf4ab9 100644 (file)
--- a/src/basic/fileio.c
+++ b/src/basic/fileio.c
@@ -30,6 +30,7 @@
 
 #include "alloc-util.h"
 #include "ctype.h"
+#include "env-util.h"
 #include "escape.h"
 #include "fd-util.h"
 #include "fileio.h"
@@ -678,13 +679,15 @@ static int load_env_file_push(
         if (r < 0)
                 return r;
 
-        p = strjoin(key, "=", strempty(value));
+        p = strjoin(key, "=", value);
         if (!p)
                 return -ENOMEM;
 
-        r = strv_consume(m, p);
-        if (r < 0)
+        r = strv_env_replace(m, p);
+        if (r < 0) {
+                free(p);
                 return r;
+        }
 
         if (n_pushed)
                 (*n_pushed)++;

diff --git a/src/test/test-fileio.c b/src/test/test-fileio.c
index 56316904a38e37c4963a52c8fc2fab062f6774c2..a38bb874a91cc5365e3331718cfbe966cd639d28 100644 (file)
--- a/src/test/test-fileio.c
+++ b/src/test/test-fileio.c
@@ -71,6 +71,8 @@ static void test_parse_env_file(void) {
               "seven=\"sevenval\" #nocomment\n"
               "eight=eightval #nocomment\n"
               "export nine=nineval\n"
+              "ten=ignored\n"
+              "ten=ignored\n"
               "ten=", f);
 
         fflush(f);