[systemd-commits] 5 commits - Makefile.am src/core src/hostname src/locale src/notify src/shared src/test

Lennart Poettering lennart at kemper.freedesktop.org
Sun Feb 10 20:45:42 PST 2013


 Makefile.am                 |    2 
 src/core/dbus-manager.c     |    1 
 src/core/execute.c          |    1 
 src/core/service.c          |    1 
 src/hostname/hostnamed.c    |    1 
 src/locale/localed.c        |    1 
 src/notify/notify.c         |    1 
 src/shared/env-util.c       |  394 ++++++++++++++++++++++++++++++++++++++++++++
 src/shared/env-util.h       |   41 ++++
 src/shared/path-util.c      |    7 
 src/shared/strv.c           |  243 ---------------------------
 src/shared/strv.h           |   11 -
 src/shared/util.c           |  117 ++++++-------
 src/shared/util.h           |   10 +
 src/test/test-env-replace.c |  127 +++++++++-----
 15 files changed, 606 insertions(+), 352 deletions(-)

New commits:
commit dfbacb6fe56db4820137e4b668c361168fbfe5ff
Author: Lennart Poettering <lennart at poettering.net>
Date:   Mon Feb 11 05:36:54 2013 +0100

    util: rework load_env_file()
    
    Inner library calls should not invoke log_oom(), that's something for
    main programs, not library calls.
    
    Don't read through uninitialized memory if a file ends in a continuation
    line.
    
    Add comments for the non-obvious bits.
    
    Don't choke on comment lines that are continuation lines.
    
    Simplify some things.

diff --git a/src/shared/util.c b/src/shared/util.c
index 1d30ea5..29cb9f1 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -766,77 +766,82 @@ fail:
         return r;
 }
 
-int load_env_file(const char *fname,
-                  char ***rl) {
+int load_env_file(const char *fname, char ***rl) {
 
-        FILE _cleanup_fclose_ *f;
-        char *b;
-        char _cleanup_free_ *c = NULL;
-        char _cleanup_strv_free_ **m = NULL;
+        _cleanup_fclose_ FILE *f;
+        _cleanup_strv_free_ char **m = NULL;
+        _cleanup_free_ char *c = NULL;
 
         assert(fname);
         assert(rl);
 
+        /* This reads an environment file, but will not complain about
+         * any invalid assignments, that needs to be done by the
+         * caller */
+
         f = fopen(fname, "re");
         if (!f)
                 return -errno;
 
         while (!feof(f)) {
-                char l[LINE_MAX], *p, *u, *cs;
-                char **t;
+                char l[LINE_MAX], *p, *cs, *b;
 
                 if (!fgets(l, sizeof(l), f)) {
-                        if (!feof(f))
+                        if (ferror(f))
                                 return -errno;
-                        else if (!c)
-                                break;
+
+                        /* The previous line was a continuation line?
+                         * Let's process it now, before we leave the
+                         * loop */
+                        if (c)
+                                goto process;
+
+                        break;
                 }
 
+                /* Is this a continuation line? If so, just append
+                 * this to c, and go to next line right-away */
                 cs = endswith(l, "\\\n");
                 if (cs) {
                         *cs = '\0';
                         b = strappend(c, l);
                         if (!b)
-                                return log_oom();
+                                return -ENOMEM;
 
                         free(c);
                         c = b;
-                        *l = '\0';
                         continue;
                 }
 
+                /* If the previous line was a continuation line,
+                 * append the current line to it */
                 if (c) {
                         b = strappend(c, l);
                         if (!b)
-                                return log_oom();
+                                return -ENOMEM;
 
                         free(c);
                         c = b;
                 }
 
+        process:
                 p = strstrip(c ? c : l);
 
-                if (!*p)
-                        continue;
+                if (*p && !strchr(COMMENTS, *p)) {
+                        _cleanup_free_ char *u;
+                        int k;
 
-                if (strchr(COMMENTS, *p))
-                        continue;
+                        u = normalize_env_assignment(p);
+                        if (!u)
+                                return -ENOMEM;
 
-                u = normalize_env_assignment(p);
-                if (!u)
-                        return log_oom();
+                        k = strv_extend(&m, u);
+                        if (k < 0)
+                                return -ENOMEM;
+                }
 
                 free(c);
                 c = NULL;
-
-                t = strv_append(m, u);
-                free(u);
-
-                if (!t)
-                        return log_oom();
-
-                strv_free(m);
-                m = t;
         }
 
         *rl = m;

commit 91a6489d9949776605939fe65a2a6174ee719049
Author: Lennart Poettering <lennart at poettering.net>
Date:   Mon Feb 11 05:10:23 2013 +0100

    path-util: fix memory leak

diff --git a/src/shared/path-util.c b/src/shared/path-util.c
index ae12c05..52ce65d 100644
--- a/src/shared/path-util.c
+++ b/src/shared/path-util.c
@@ -135,7 +135,8 @@ char *path_make_absolute_cwd(const char *p) {
         if (path_is_absolute(p))
                 return strdup(p);
 
-        if (!(cwd = get_current_dir_name()))
+        cwd = get_current_dir_name();
+        if (!cwd)
                 return NULL;
 
         r = path_make_absolute(p, cwd);
@@ -190,7 +191,6 @@ char **path_strv_canonicalize(char **l) {
 
                 errno = 0;
                 u = canonicalize_file_name(t);
-
                 if (!u) {
                         if (errno == ENOENT)
                                 u = t;
@@ -201,7 +201,8 @@ char **path_strv_canonicalize(char **l) {
 
                                 continue;
                         }
-                }
+                } else
+                        free(t);
 
                 l[k++] = u;
         }

commit f74e605fc06c1c23e968dc4c26045eb746791706
Author: Lennart Poettering <lennart at poettering.net>
Date:   Mon Feb 11 05:09:29 2013 +0100

    util: introduce FOREACH_LINE for iterating through files

diff --git a/src/shared/util.c b/src/shared/util.c
index 5b795d4..1d30ea5 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -1063,10 +1063,10 @@ int get_process_exe(pid_t pid, char **name) {
 }
 
 static int get_process_id(pid_t pid, const char *field, uid_t *uid) {
-        char *p;
-        FILE *f;
-        int r;
+        _cleanup_fclose_ FILE *f = NULL;
+        _cleanup_free_ char *p = NULL;
 
+        assert(field);
         assert(uid);
 
         if (pid == 0)
@@ -1076,21 +1076,11 @@ static int get_process_id(pid_t pid, const char *field, uid_t *uid) {
                 return -ENOMEM;
 
         f = fopen(p, "re");
-        free(p);
-
         if (!f)
                 return -errno;
 
-        while (!feof(f)) {
-                char line[LINE_MAX], *l;
-
-                if (!fgets(line, sizeof(line), f)) {
-                        if (feof(f))
-                                break;
-
-                        r = -errno;
-                        goto finish;
-                }
+        FOREACH_LINE(f, line, return -errno) {
+                char *l;
 
                 l = strstrip(line);
 
@@ -1100,17 +1090,11 @@ static int get_process_id(pid_t pid, const char *field, uid_t *uid) {
 
                         l[strcspn(l, WHITESPACE)] = 0;
 
-                        r = parse_uid(l, uid);
-                        goto finish;
+                        return parse_uid(l, uid);
                 }
         }
 
-        r = -EIO;
-
-finish:
-        fclose(f);
-
-        return r;
+        return -EIO;
 }
 
 int get_process_uid(pid_t pid, uid_t *uid) {
diff --git a/src/shared/util.h b/src/shared/util.h
index 18494f1..d926b01 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -567,3 +567,12 @@ char *strreplace(const char *text, const char *old_string, const char *new_strin
 char *strip_tab_ansi(char **p, size_t *l);
 
 int on_ac_power(void);
+
+#define FOREACH_LINE(f, line, on_error)                         \
+        for (char line[LINE_MAX]; !feof(f); )                   \
+                if (!fgets(line, sizeof(line), f)) {            \
+                        if (ferror(f)) {                        \
+                                on_error;                       \
+                        }                                       \
+                        break;                                  \
+                } else

commit 9b5d6bd909855543cba75a4469bff6f82845cf0d
Author: Lennart Poettering <lennart at poettering.net>
Date:   Mon Feb 11 03:52:32 2013 +0100

    test-env-replace: better user assert_se() everywhere
    
    For test tools it's better to use assert_se() rather than assert(),
    since the former is not optimized away, even if -DNDEBUG is used. That
    means the test program now retains its usefulness even if -DNDEBUG is
    used.
    
    This also adds in some OOM checks, to be extra anal...

diff --git a/src/test/test-env-replace.c b/src/test/test-env-replace.c
index b8747db..d6cb289 100644
--- a/src/test/test-env-replace.c
+++ b/src/test/test-env-replace.c
@@ -30,64 +30,77 @@ static void test_strv_env_delete(void) {
         _cleanup_strv_free_ char **a = NULL, **b = NULL, **c = NULL, **d = NULL;
 
         a = strv_new("FOO=BAR", "WALDO=WALDO", "WALDO=", "PIEP", "SCHLUMPF=SMURF", NULL);
+        assert_se(a);
+
         b = strv_new("PIEP", "FOO", NULL);
+        assert_se(b);
+
         c = strv_new("SCHLUMPF", NULL);
+        assert_se(c);
 
         d = strv_env_delete(a, 2, b, c);
+        assert_se(d);
 
-        assert(streq(d[0], "WALDO=WALDO"));
-        assert(streq(d[1], "WALDO="));
-        assert(strv_length(d) == 2);
+        assert_se(streq(d[0], "WALDO=WALDO"));
+        assert_se(streq(d[1], "WALDO="));
+        assert_se(strv_length(d) == 2);
 }
 
 static void test_strv_env_unset(void) {
         _cleanup_strv_free_ char **l = NULL;
 
         l = strv_new("PIEP", "SCHLUMPF=SMURFF", "NANANANA=YES", NULL);
+        assert_se(l);
 
-        strv_env_unset(l, "SCHLUMPF");
+        assert_se(strv_env_unset(l, "SCHLUMPF") == l);
 
-        assert(streq(l[0], "PIEP"));
-        assert(streq(l[1], "NANANANA=YES"));
-        assert(strv_length(l) == 2);
+        assert_se(streq(l[0], "PIEP"));
+        assert_se(streq(l[1], "NANANANA=YES"));
+        assert_se(strv_length(l) == 2);
 }
 
 static void test_strv_env_set(void) {
         _cleanup_strv_free_ char **l = NULL, **r = NULL;
 
         l = strv_new("PIEP", "SCHLUMPF=SMURFF", "NANANANA=YES", NULL);
+        assert_se(l);
 
         r = strv_env_set(l, "WALDO=WALDO");
+        assert_se(r);
 
-        assert(streq(r[0], "PIEP"));
-        assert(streq(r[1], "SCHLUMPF=SMURFF"));
-        assert(streq(r[2], "NANANANA=YES"));
-        assert(streq(r[3], "WALDO=WALDO"));
-        assert(strv_length(r) == 4);
+        assert_se(streq(r[0], "PIEP"));
+        assert_se(streq(r[1], "SCHLUMPF=SMURFF"));
+        assert_se(streq(r[2], "NANANANA=YES"));
+        assert_se(streq(r[3], "WALDO=WALDO"));
+        assert_se(strv_length(r) == 4);
 }
 
 static void test_strv_env_merge(void) {
         _cleanup_strv_free_ char **a = NULL, **b = NULL, **r = NULL;
 
         a = strv_new("FOO=BAR", "WALDO=WALDO", "WALDO=", "PIEP", "SCHLUMPF=SMURF", NULL);
+        assert_se(a);
+
         b = strv_new("FOO=KKK", "FOO=", "PIEP=", "SCHLUMPF=SMURFF", "NANANANA=YES", NULL);
+        assert_se(b);
 
         r = strv_env_merge(2, a, b);
-        assert(streq(r[0], "FOO="));
-        assert(streq(r[1], "WALDO="));
-        assert(streq(r[2], "PIEP"));
-        assert(streq(r[3], "SCHLUMPF=SMURFF"));
-        assert(streq(r[4], "PIEP="));
-        assert(streq(r[5], "NANANANA=YES"));
-        assert(strv_length(r) == 6);
-
-        strv_env_clean(r);
-        assert(streq(r[0], "FOO="));
-        assert(streq(r[1], "WALDO="));
-        assert(streq(r[2], "SCHLUMPF=SMURFF"));
-        assert(streq(r[3], "PIEP="));
-        assert(streq(r[4], "NANANANA=YES"));
-        assert(strv_length(r) == 5);
+        assert_se(r);
+        assert_se(streq(r[0], "FOO="));
+        assert_se(streq(r[1], "WALDO="));
+        assert_se(streq(r[2], "PIEP"));
+        assert_se(streq(r[3], "SCHLUMPF=SMURFF"));
+        assert_se(streq(r[4], "PIEP="));
+        assert_se(streq(r[5], "NANANANA=YES"));
+        assert_se(strv_length(r) == 6);
+
+        assert_se(strv_env_clean(r) == r);
+        assert_se(streq(r[0], "FOO="));
+        assert_se(streq(r[1], "WALDO="));
+        assert_se(streq(r[2], "SCHLUMPF=SMURFF"));
+        assert_se(streq(r[3], "PIEP="));
+        assert_se(streq(r[4], "NANANANA=YES"));
+        assert_se(strv_length(r) == 5);
 }
 
 static void test_replace_env_arg(void) {
@@ -111,24 +124,25 @@ static void test_replace_env_arg(void) {
         _cleanup_strv_free_ char **r = NULL;
 
         r = replace_env_argv((char**) line, (char**) env);
-        assert(streq(r[0], "FOO$FOO"));
-        assert(streq(r[1], "FOO$FOOFOO"));
-        assert(streq(r[2], "FOOBAR BAR$FOO"));
-        assert(streq(r[3], "FOOBAR BAR"));
-        assert(streq(r[4], "BAR BAR"));
-        assert(streq(r[5], "BAR"));
-        assert(streq(r[6], "BAR"));
-        assert(streq(r[7], "BAR BARwaldo"));
-        assert(streq(r[8], "${FOO"));
-        assert(strv_length(r) == 9);
+        assert_se(r);
+        assert_se(streq(r[0], "FOO$FOO"));
+        assert_se(streq(r[1], "FOO$FOOFOO"));
+        assert_se(streq(r[2], "FOOBAR BAR$FOO"));
+        assert_se(streq(r[3], "FOOBAR BAR"));
+        assert_se(streq(r[4], "BAR BAR"));
+        assert_se(streq(r[5], "BAR"));
+        assert_se(streq(r[6], "BAR"));
+        assert_se(streq(r[7], "BAR BARwaldo"));
+        assert_se(streq(r[8], "${FOO"));
+        assert_se(strv_length(r) == 9);
 }
 
-static void test_one_normalize(const char *input, const char *output)
-{
+static void test_one_normalize(const char *input, const char *output) {
         _cleanup_free_ char *t;
 
         t = normalize_env_assignment(input);
-        assert(streq(t, output));
+        assert_se(t);
+        assert_se(streq(t, output));
 }
 
 static void test_normalize_env_assignment(void) {
@@ -149,7 +163,6 @@ static void test_normalize_env_assignment(void) {
 }
 
 static void test_env_clean(void) {
-
         _cleanup_strv_free_ char **e;
 
         e = strv_new("FOOBAR=WALDO",
@@ -169,8 +182,10 @@ static void test_env_clean(void) {
                      "another=one",
                      "another=final one",
                      NULL);
-
-        assert_se(strv_env_clean(e));
+        assert_se(e);
+        assert_se(!strv_env_is_valid(e));
+        assert_se(strv_env_clean(e) == e);
+        assert_se(strv_env_is_valid(e));
 
         assert_se(streq(e[0], "FOOBAR=WALDO"));
         assert_se(streq(e[1], "X="));

commit 4d1a69043862ed979642f5688097160355d4cc81
Author: Lennart Poettering <lennart at poettering.net>
Date:   Mon Feb 11 03:46:08 2013 +0100

    env: considerably beef up environment cleaning logic
    
    Now, actually check if the environment variable names and values used
    are valid, before accepting them. With this in place are at some places
    more rigid than POSIX, and less rigid at others. For example, this code
    allows lower-case environment variables (which POSIX suggests not to
    use), but it will not allow non-UTF8 variable values.
    
    All in all this should be a good middle ground of what to allow and what
    not to allow as environment variables.
    
    (This also splits out all environment related calls into env-util.[ch])

diff --git a/Makefile.am b/Makefile.am
index 8a7c2b6..9da3394 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -573,6 +573,8 @@ libsystemd_shared_la_SOURCES = \
 	src/shared/fdset.h \
 	src/shared/strv.c \
 	src/shared/strv.h \
+	src/shared/env-util.c \
+	src/shared/env-util.h \
 	src/shared/strbuf.c \
 	src/shared/strbuf.h \
 	src/shared/strxcpyx.c \
diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
index b782957..7071196 100644
--- a/src/core/dbus-manager.c
+++ b/src/core/dbus-manager.c
@@ -36,6 +36,7 @@
 #include "path-util.h"
 #include "dbus-unit.h"
 #include "virt.h"
+#include "env-util.h"
 
 #define BUS_MANAGER_INTERFACE_BEGIN                                     \
         " <interface name=\"org.freedesktop.systemd1.Manager\">\n"
diff --git a/src/core/execute.c b/src/core/execute.c
index 1413c91..aa58bc4 100644
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -64,6 +64,7 @@
 #include "loopback-setup.h"
 #include "path-util.h"
 #include "syscall-list.h"
+#include "env-util.h"
 
 #define IDLE_TIMEOUT_USEC (5*USEC_PER_SEC)
 
diff --git a/src/core/service.c b/src/core/service.c
index 9141463..9c4bc41 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -42,6 +42,7 @@
 #include "path-util.h"
 #include "util.h"
 #include "utf8.h"
+#include "env-util.h"
 
 #ifdef HAVE_SYSV_COMPAT
 
diff --git a/src/hostname/hostnamed.c b/src/hostname/hostnamed.c
index 92b150b..c5a8b6f 100644
--- a/src/hostname/hostnamed.c
+++ b/src/hostname/hostnamed.c
@@ -32,6 +32,7 @@
 #include "polkit.h"
 #include "def.h"
 #include "virt.h"
+#include "env-util.h"
 
 #define INTERFACE \
         " <interface name=\"org.freedesktop.hostname1\">\n"             \
diff --git a/src/locale/localed.c b/src/locale/localed.c
index bb2a3a2..6b1a793 100644
--- a/src/locale/localed.c
+++ b/src/locale/localed.c
@@ -31,6 +31,7 @@
 #include "dbus-common.h"
 #include "polkit.h"
 #include "def.h"
+#include "env-util.h"
 
 #define INTERFACE                                                       \
         " <interface name=\"org.freedesktop.locale1\">\n"               \
diff --git a/src/notify/notify.c b/src/notify/notify.c
index f521f56..1e9766f 100644
--- a/src/notify/notify.c
+++ b/src/notify/notify.c
@@ -34,6 +34,7 @@
 #include "log.h"
 #include "sd-readahead.h"
 #include "build.h"
+#include "env-util.h"
 
 static bool arg_ready = false;
 static pid_t arg_pid = 0;
diff --git a/src/shared/env-util.c b/src/shared/env-util.c
new file mode 100644
index 0000000..7a213a7
--- /dev/null
+++ b/src/shared/env-util.c
@@ -0,0 +1,394 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+  This file is part of systemd.
+
+  Copyright 2012 Lennart Poettering
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+#include <limits.h>
+#include <sys/param.h>
+#include <unistd.h>
+
+#include "strv.h"
+#include "utf8.h"
+#include "util.h"
+#include "env-util.h"
+
+#define VALID_CHARS_ENV_NAME                    \
+        "0123456789"                            \
+        "abcdefghijklmnopqrstuvwxyz"            \
+        "ABCDEFGHIJKLMNOPQRSTUVWXYZ"            \
+        "_"
+
+#ifndef ARG_MAX
+#define ARG_MAX ((size_t) sysconf(_SC_ARG_MAX))
+#endif
+
+static bool env_name_is_valid_n(const char *e, size_t n) {
+        const char *p;
+
+        if (!e)
+                return false;
+
+        if (n <= 0)
+                return false;
+
+        if (e[0] >= '0' && e[0] <= '9')
+                return false;
+
+        /* POSIX says the overall size of the environment block cannot
+         * be > ARG_MAX, an individual assignment hence cannot be
+         * either. Discounting the equal sign and trailing NUL this
+         * hence leaves ARG_MAX-2 as longest possible variable
+         * name. */
+        if (n > ARG_MAX - 2)
+                return false;
+
+        for (p = e; p < e + n; p++)
+                if (!strchr(VALID_CHARS_ENV_NAME, *p))
+                        return false;
+
+        return true;
+}
+
+bool env_name_is_valid(const char *e) {
+        if (!e)
+                return false;
+
+        return env_name_is_valid_n(e, strlen(e));
+}
+
+bool env_value_is_valid(const char *e) {
+        if (!e)
+                return false;
+
+        if (!utf8_is_valid(e))
+                return false;
+
+        if (string_has_cc(e))
+                return false;
+
+        /* POSIX says the overall size of the environment block cannot
+         * be > ARG_MAX, an individual assignment hence cannot be
+         * either. Discounting the shortest possible variable name of
+         * length 1, the equal sign and trailing NUL this hence leaves
+         * ARG_MAX-3 as longest possible variable value. */
+        if (strlen(e) > ARG_MAX - 3)
+                return false;
+
+        return true;
+}
+
+bool env_assignment_is_valid(const char *e) {
+        const char *eq;
+
+        eq = strchr(e, '=');
+        if (!eq)
+                return false;
+
+        if (!env_name_is_valid_n(e, eq - e))
+                return false;
+
+        if (!env_value_is_valid(eq + 1))
+                return false;
+
+        /* POSIX says the overall size of the environment block cannot
+         * be > ARG_MAX, hence the individual variable assignments
+         * cannot be either, but let's room for one trailing NUL
+         * byte. */
+        if (strlen(e) > ARG_MAX - 1)
+                return false;
+
+        return true;
+}
+
+bool strv_env_is_valid(char **e) {
+        char **p, **q;
+
+        STRV_FOREACH(p, e) {
+                size_t k;
+
+                if (!env_assignment_is_valid(*p))
+                        return false;
+
+                /* Check if there are duplicate assginments */
+                k = strcspn(*p, "=");
+                STRV_FOREACH(q, p + 1)
+                        if (strncmp(*p, *q, k) == 0 && (*q)[k] == '=')
+                                return false;
+        }
+
+        return true;
+}
+
+static int env_append(char **r, char ***k, char **a) {
+        assert(r);
+        assert(k);
+
+        if (!a)
+                return 0;
+
+        /* Add the entries of a to *k unless they already exist in *r
+         * in which case they are overridden instead. This assumes
+         * there is enough space in the r array. */
+
+        for (; *a; a++) {
+                char **j;
+                size_t n;
+
+                n = strcspn(*a, "=");
+
+                if ((*a)[n] == '=')
+                        n++;
+
+                for (j = r; j < *k; j++)
+                        if (strncmp(*j, *a, n) == 0)
+                                break;
+
+                if (j >= *k)
+                        (*k)++;
+                else
+                        free(*j);
+
+                *j = strdup(*a);
+                if (!*j)
+                        return -ENOMEM;
+        }
+
+        return 0;
+}
+
+char **strv_env_merge(unsigned n_lists, ...) {
+        size_t n = 0;
+        char **l, **k, **r;
+        va_list ap;
+        unsigned i;
+
+        /* Merges an arbitrary number of environment sets */
+
+        va_start(ap, n_lists);
+        for (i = 0; i < n_lists; i++) {
+                l = va_arg(ap, char**);
+                n += strv_length(l);
+        }
+        va_end(ap);
+
+        r = new(char*, n+1);
+        if (!r)
+                return NULL;
+
+        k = r;
+
+        va_start(ap, n_lists);
+        for (i = 0; i < n_lists; i++) {
+                l = va_arg(ap, char**);
+                if (env_append(r, &k, l) < 0)
+                        goto fail;
+        }
+        va_end(ap);
+
+        *k = NULL;
+
+        return r;
+
+fail:
+        va_end(ap);
+        strv_free(r);
+
+        return NULL;
+}
+
+static bool env_match(const char *t, const char *pattern) {
+        assert(t);
+        assert(pattern);
+
+        /* pattern a matches string a
+         *         a matches a=
+         *         a matches a=b
+         *         a= matches a=
+         *         a=b matches a=b
+         *         a= does not match a
+         *         a=b does not match a=
+         *         a=b does not match a
+         *         a=b does not match a=c */
+
+        if (streq(t, pattern))
+                return true;
+
+        if (!strchr(pattern, '=')) {
+                size_t l = strlen(pattern);
+
+                return strncmp(t, pattern, l) == 0 && t[l] == '=';
+        }
+
+        return false;
+}
+
+char **strv_env_delete(char **x, unsigned n_lists, ...) {
+        size_t n, i = 0;
+        char **k, **r;
+        va_list ap;
+
+        /* Deletes every entry from x that is mentioned in the other
+         * string lists */
+
+        n = strv_length(x);
+
+        r = new(char*, n+1);
+        if (!r)
+                return NULL;
+
+        STRV_FOREACH(k, x) {
+                unsigned v;
+
+                va_start(ap, n_lists);
+                for (v = 0; v < n_lists; v++) {
+                        char **l, **j;
+
+                        l = va_arg(ap, char**);
+                        STRV_FOREACH(j, l)
+                                if (env_match(*k, *j))
+                                        goto skip;
+                }
+                va_end(ap);
+
+                r[i] = strdup(*k);
+                if (!r[i]) {
+                        strv_free(r);
+                        return NULL;
+                }
+
+                i++;
+                continue;
+
+        skip:
+                va_end(ap);
+        }
+
+        r[i] = NULL;
+
+        assert(i <= n);
+
+        return r;
+}
+
+char **strv_env_unset(char **l, const char *p) {
+
+        char **f, **t;
+
+        if (!l)
+                return NULL;
+
+        assert(p);
+
+        /* Drops every occurrence of the env var setting p in the
+         * string list. edits in-place. */
+
+        for (f = t = l; *f; f++) {
+
+                if (env_match(*f, p)) {
+                        free(*f);
+                        continue;
+                }
+
+                *(t++) = *f;
+        }
+
+        *t = NULL;
+        return l;
+}
+
+char **strv_env_set(char **x, const char *p) {
+
+        char **k, **r;
+        char* m[2] = { (char*) p, NULL };
+
+        /* Overrides the env var setting of p, returns a new copy */
+
+        r = new(char*, strv_length(x)+2);
+        if (!r)
+                return NULL;
+
+        k = r;
+        if (env_append(r, &k, x) < 0)
+                goto fail;
+
+        if (env_append(r, &k, m) < 0)
+                goto fail;
+
+        *k = NULL;
+
+        return r;
+
+fail:
+        strv_free(r);
+        return NULL;
+}
+
+char *strv_env_get_n(char **l, const char *name, size_t k) {
+        char **i;
+
+        assert(name);
+
+        if (k <= 0)
+                return NULL;
+
+        STRV_FOREACH(i, l)
+                if (strncmp(*i, name, k) == 0 &&
+                    (*i)[k] == '=')
+                        return *i + k + 1;
+
+        return NULL;
+}
+
+char *strv_env_get(char **l, const char *name) {
+        assert(name);
+
+        return strv_env_get_n(l, name, strlen(name));
+}
+
+char **strv_env_clean(char **e) {
+        char **p, **q;
+        int k = 0;
+
+        STRV_FOREACH(p, e) {
+                size_t n;
+                bool duplicate = false;
+
+                if (!env_assignment_is_valid(*p)) {
+                        free(*p);
+                        continue;
+                }
+
+                n = strcspn(*p, "=");
+                STRV_FOREACH(q, p + 1)
+                        if (strncmp(*p, *q, n) == 0 && (*q)[n] == '=') {
+                                duplicate = true;
+                                break;
+                        }
+
+                if (duplicate) {
+                        free(*p);
+                        continue;
+                }
+
+                e[k++] = *p;
+        }
+
+        e[k] = NULL;
+        return e;
+}
diff --git a/src/shared/env-util.h b/src/shared/env-util.h
new file mode 100644
index 0000000..93bf596
--- /dev/null
+++ b/src/shared/env-util.h
@@ -0,0 +1,41 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+#pragma once
+
+/***
+  This file is part of systemd.
+
+  Copyright 2013 Lennart Poettering
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+#include <stdbool.h>
+#include <sys/types.h>
+
+bool env_name_is_valid(const char *e);
+bool env_value_is_valid(const char *e);
+bool env_assignment_is_valid(const char *e);
+
+bool strv_env_is_valid(char **e);
+char **strv_env_clean(char **l);
+
+char **strv_env_merge(unsigned n_lists, ...);
+char **strv_env_delete(char **x, unsigned n_lists, ...); /* New copy */
+
+char **strv_env_set(char **x, const char *p); /* New copy ... */
+char **strv_env_unset(char **l, const char *p); /* In place ... */
+
+char *strv_env_get_n(char **l, const char *name, size_t k);
+char *strv_env_get(char **x, const char *n);
diff --git a/src/shared/strv.c b/src/shared/strv.c
index fc6104f..ee0b71e 100644
--- a/src/shared/strv.c
+++ b/src/shared/strv.c
@@ -458,249 +458,6 @@ char **strv_remove_prefix(char **l, const char *s) {
         return l;
 }
 
-static int env_append(char **r, char ***k, char **a) {
-        assert(r);
-        assert(k);
-
-        if (!a)
-                return 0;
-
-        /* Add the entries of a to *k unless they already exist in *r
-         * in which case they are overridden instead. This assumes
-         * there is enough space in the r array. */
-
-        for (; *a; a++) {
-                char **j;
-                size_t n;
-
-                n = strcspn(*a, "=");
-
-                if ((*a)[n] == '=')
-                        n++;
-
-                for (j = r; j < *k; j++)
-                        if (strncmp(*j, *a, n) == 0)
-                                break;
-
-                if (j >= *k)
-                        (*k)++;
-                else
-                        free(*j);
-
-                *j = strdup(*a);
-                if (!*j)
-                        return -ENOMEM;
-        }
-
-        return 0;
-}
-
-char **strv_env_merge(unsigned n_lists, ...) {
-        size_t n = 0;
-        char **l, **k, **r;
-        va_list ap;
-        unsigned i;
-
-        /* Merges an arbitrary number of environment sets */
-
-        va_start(ap, n_lists);
-        for (i = 0; i < n_lists; i++) {
-                l = va_arg(ap, char**);
-                n += strv_length(l);
-        }
-        va_end(ap);
-
-        r = new(char*, n+1);
-        if (!r)
-                return NULL;
-
-        k = r;
-
-        va_start(ap, n_lists);
-        for (i = 0; i < n_lists; i++) {
-                l = va_arg(ap, char**);
-                if (env_append(r, &k, l) < 0)
-                        goto fail;
-        }
-        va_end(ap);
-
-        *k = NULL;
-
-        return r;
-
-fail:
-        va_end(ap);
-        strv_free(r);
-
-        return NULL;
-}
-
-static bool env_match(const char *t, const char *pattern) {
-        assert(t);
-        assert(pattern);
-
-        /* pattern a matches string a
-         *         a matches a=
-         *         a matches a=b
-         *         a= matches a=
-         *         a=b matches a=b
-         *         a= does not match a
-         *         a=b does not match a=
-         *         a=b does not match a
-         *         a=b does not match a=c */
-
-        if (streq(t, pattern))
-                return true;
-
-        if (!strchr(pattern, '=')) {
-                size_t l = strlen(pattern);
-
-                return strncmp(t, pattern, l) == 0 && t[l] == '=';
-        }
-
-        return false;
-}
-
-char **strv_env_delete(char **x, unsigned n_lists, ...) {
-        size_t n, i = 0;
-        char **k, **r;
-        va_list ap;
-
-        /* Deletes every entry from x that is mentioned in the other
-         * string lists */
-
-        n = strv_length(x);
-
-        r = new(char*, n+1);
-        if (!r)
-                return NULL;
-
-        STRV_FOREACH(k, x) {
-                unsigned v;
-
-                va_start(ap, n_lists);
-                for (v = 0; v < n_lists; v++) {
-                        char **l, **j;
-
-                        l = va_arg(ap, char**);
-                        STRV_FOREACH(j, l)
-                                if (env_match(*k, *j))
-                                        goto skip;
-                }
-                va_end(ap);
-
-                r[i] = strdup(*k);
-                if (!r[i]) {
-                        strv_free(r);
-                        return NULL;
-                }
-
-                i++;
-                continue;
-
-        skip:
-                va_end(ap);
-        }
-
-        r[i] = NULL;
-
-        assert(i <= n);
-
-        return r;
-}
-
-char **strv_env_unset(char **l, const char *p) {
-
-        char **f, **t;
-
-        if (!l)
-                return NULL;
-
-        assert(p);
-
-        /* Drops every occurrence of the env var setting p in the
-         * string list. edits in-place. */
-
-        for (f = t = l; *f; f++) {
-
-                if (env_match(*f, p)) {
-                        free(*f);
-                        continue;
-                }
-
-                *(t++) = *f;
-        }
-
-        *t = NULL;
-        return l;
-}
-
-char **strv_env_set(char **x, const char *p) {
-
-        char **k, **r;
-        char* m[2] = { (char*) p, NULL };
-
-        /* Overrides the env var setting of p, returns a new copy */
-
-        r = new(char*, strv_length(x)+2);
-        if (!r)
-                return NULL;
-
-        k = r;
-        if (env_append(r, &k, x) < 0)
-                goto fail;
-
-        if (env_append(r, &k, m) < 0)
-                goto fail;
-
-        *k = NULL;
-
-        return r;
-
-fail:
-        strv_free(r);
-        return NULL;
-
-}
-
-char *strv_env_get_with_length(char **l, const char *name, size_t k) {
-        char **i;
-
-        assert(name);
-
-        STRV_FOREACH(i, l)
-                if (strncmp(*i, name, k) == 0 &&
-                    (*i)[k] == '=')
-                        return *i + k + 1;
-
-        return NULL;
-}
-
-char *strv_env_get(char **l, const char *name) {
-        return strv_env_get_with_length(l, name, strlen(name));
-}
-
-char **strv_env_clean(char **l) {
-        char **r, **ret;
-
-        for (r = ret = l; *l; l++) {
-                const char *equal;
-
-                equal = strchr(*l, '=');
-
-                if (equal && equal[1] == 0) {
-                        free(*l);
-                        continue;
-                }
-
-                *(r++) = *l;
-        }
-
-        *r = NULL;
-
-        return ret;
-}
-
 char **strv_parse_nulstr(const char *s, size_t l) {
         const char *p;
         unsigned c = 0, i = 0;
diff --git a/src/shared/strv.h b/src/shared/strv.h
index 33c752a..d28625b 100644
--- a/src/shared/strv.h
+++ b/src/shared/strv.h
@@ -61,17 +61,6 @@ char **strv_split_quoted(const char *s) _malloc_;
 
 char *strv_join(char **l, const char *separator) _malloc_;
 
-char **strv_env_merge(unsigned n_lists, ...);
-char **strv_env_delete(char **x, unsigned n_lists, ...);
-
-char **strv_env_set(char **x, const char *p);
-char **strv_env_unset(char **l, const char *p);
-
-char *strv_env_get_with_length(char **l, const char *name, size_t k);
-char *strv_env_get(char **x, const char *n);
-
-char **strv_env_clean(char **l);
-
 char **strv_parse_nulstr(const char *s, size_t l);
 
 bool strv_overlap(char **a, char **b);
diff --git a/src/shared/util.c b/src/shared/util.c
index 969ef2b..5b795d4 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -70,6 +70,7 @@
 #include "path-util.h"
 #include "exit-status.h"
 #include "hashmap.h"
+#include "env-util.h"
 
 int saved_argc = 0;
 char **saved_argv = NULL;
@@ -3341,10 +3342,10 @@ char *replace_env(const char *format, char **env) {
                         if (*e == '}') {
                                 const char *t;
 
-                                if (!(t = strv_env_get_with_length(env, word+2, e-word-2)))
-                                        t = "";
+                                t = strempty(strv_env_get_n(env, word+2, e-word-2));
 
-                                if (!(k = strappend(r, t)))
+                                k = strappend(r, t);
+                                if (!k)
                                         goto fail;
 
                                 free(r);
@@ -3385,7 +3386,8 @@ char **replace_env_argv(char **argv, char **env) {
                         char **w, **m;
                         unsigned q;
 
-                        if ((e = strv_env_get(env, *i+1))) {
+                        e = strv_env_get(env, *i+1);
+                        if (e) {
 
                                 if (!(m = strv_split_quoted(e))) {
                                         r[k] = NULL;
@@ -5608,6 +5610,18 @@ bool string_is_safe(const char *p) {
         return true;
 }
 
+bool string_has_cc(const char *p) {
+        const char *t;
+
+        assert(p);
+
+        for (t = p; *t; t++)
+                if (*t > 0 && *t < ' ')
+                        return true;
+
+        return false;
+}
+
 bool path_is_safe(const char *p) {
 
         if (isempty(p))
diff --git a/src/shared/util.h b/src/shared/util.h
index 223617c..18494f1 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -544,6 +544,7 @@ _malloc_ static inline void *memdup_multiply(const void *p, size_t a, size_t b)
 bool filename_is_safe(const char *p);
 bool path_is_safe(const char *p);
 bool string_is_safe(const char *p);
+bool string_has_cc(const char *p);
 
 void *xbsearch_r(const void *key, const void *base, size_t nmemb, size_t size,
                  int (*compar) (const void *, const void *, void *),
diff --git a/src/test/test-env-replace.c b/src/test/test-env-replace.c
index 2da3845..b8747db 100644
--- a/src/test/test-env-replace.c
+++ b/src/test/test-env-replace.c
@@ -24,6 +24,7 @@
 
 #include "util.h"
 #include "strv.h"
+#include "env-util.h"
 
 static void test_strv_env_delete(void) {
         _cleanup_strv_free_ char **a = NULL, **b = NULL, **c = NULL, **d = NULL;
@@ -81,10 +82,12 @@ static void test_strv_env_merge(void) {
         assert(strv_length(r) == 6);
 
         strv_env_clean(r);
-        assert(streq(r[0], "PIEP"));
-        assert(streq(r[1], "SCHLUMPF=SMURFF"));
-        assert(streq(r[2], "NANANANA=YES"));
-        assert(strv_length(r) == 3);
+        assert(streq(r[0], "FOO="));
+        assert(streq(r[1], "WALDO="));
+        assert(streq(r[2], "SCHLUMPF=SMURFF"));
+        assert(streq(r[3], "PIEP="));
+        assert(streq(r[4], "NANANANA=YES"));
+        assert(strv_length(r) == 5);
 }
 
 static void test_replace_env_arg(void) {
@@ -145,6 +148,38 @@ static void test_normalize_env_assignment(void) {
         test_one_normalize(" ' xyz' = 'bar ' ", "' xyz'=bar ");
 }
 
+static void test_env_clean(void) {
+
+        _cleanup_strv_free_ char **e;
+
+        e = strv_new("FOOBAR=WALDO",
+                     "FOOBAR=WALDO",
+                     "FOOBAR",
+                     "F",
+                     "X=",
+                     "F=F",
+                     "=",
+                     "=F",
+                     "",
+                     "0000=000",
+                     "äöüß=abcd",
+                     "abcd=äöüß",
+                     "xyz\n=xyz",
+                     "xyz=xyz\n",
+                     "another=one",
+                     "another=final one",
+                     NULL);
+
+        assert_se(strv_env_clean(e));
+
+        assert_se(streq(e[0], "FOOBAR=WALDO"));
+        assert_se(streq(e[1], "X="));
+        assert_se(streq(e[2], "F=F"));
+        assert_se(streq(e[3], "abcd=äöüß"));
+        assert_se(streq(e[4], "another=final one"));
+        assert_se(e[5] == NULL);
+}
+
 int main(int argc, char *argv[]) {
         test_strv_env_delete();
         test_strv_env_unset();
@@ -152,6 +187,7 @@ int main(int argc, char *argv[]) {
         test_strv_env_merge();
         test_replace_env_arg();
         test_normalize_env_assignment();
+        test_env_clean();
 
         return 0;
 }



More information about the systemd-commits mailing list