[systemd-commits] 5 commits - man/systemd.service.xml src/core src/shared src/test

Zbigniew Jędrzejewski-Szmek zbyszek at kemper.freedesktop.org
Thu Dec 18 16:33:29 PST 2014


 man/systemd.service.xml   |   93 ++++++++++++++++++++++++---
 src/core/dbus-service.c   |    6 -
 src/core/execute.c        |   10 +-
 src/core/execute.h        |    2 
 src/core/load-fragment.c  |  136 +++++++++++++++++++---------------------
 src/shared/condition.c    |    4 -
 src/shared/condition.h    |    2 
 src/shared/util.c         |    8 ++
 src/test/test-strv.c      |   30 +++++---
 src/test/test-unit-file.c |  156 ++++++++++++++++++++++++++++++++++++----------
 src/test/test-util.c      |   49 ++++++++------
 11 files changed, 339 insertions(+), 157 deletions(-)

New commits:
commit c853953658865ec3b3487e3f1eb2cc0516320bf3
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Thu Dec 18 18:08:13 2014 -0500

    load-fragment: allow quoting in command name and document allowed escapes
    
    The handling of the command name and other arguments is unified. This
    simplifies things and should make them more predictable for users.
    Incidentally, this makes ExecStart handling match the .desktop file
    specification, apart for the requirment for an absolute path.
    
    https://bugs.freedesktop.org/show_bug.cgi?id=86171

diff --git a/man/systemd.service.xml b/man/systemd.service.xml
index da9079c..0b68aa0 100644
--- a/man/systemd.service.xml
+++ b/man/systemd.service.xml
@@ -377,11 +377,10 @@
 
                                 <para>For each of the specified
                                 commands, the first argument must be
-                                an absolute and literal path to an
-                                executable. Optionally, if the
-                                absolute file name is prefixed with
-                                <literal>@</literal>, the second token
-                                will be passed as
+                                an absolute path to an executable.
+                                Optionally, if this file name is
+                                prefixed with <literal>@</literal>,
+                                the second token will be passed as
                                 <literal>argv[0]</literal> to the
                                 executed process, followed by the
                                 further arguments specified. If the
@@ -1148,13 +1147,15 @@
 
                 <para>Each command line is split on whitespace, with
                 the first item being the command to execute, and the
-                subsequent items being the arguments.  Double quotes
+                subsequent items being the arguments. Double quotes
                 ("...") and single quotes ('...') may be used, in
                 which case everything until the next matching quote
-                becomes part of the same argument. Quotes themselves
-                are removed after parsing. In addition, a trailing
-                backslash (<literal>\</literal>) may be used to merge
-                lines. </para>
+                becomes part of the same argument. C-style escapes are
+                also supported, see table below. Quotes themselves are
+                removed after parsing and escape sequences
+                substituted. In addition, a trailing backslash
+                (<literal>\</literal>) may be used to merge lines.
+                </para>
 
                 <para>This syntax is intended to be very similar to
                 shell syntax, but only the meta-characters and
@@ -1168,6 +1169,10 @@
                 <emphasis>other elements of shell syntax are not
                 supported</emphasis>.</para>
 
+                <para>The command to execute must an absolute path
+                name. It may contain spaces, but control characters
+                are not allowed.</para>
+
                 <para>The command line accepts <literal>%</literal>
                 specifiers as described in
                 <citerefentry><refentrytitle>systemd.unit</refentrytitle><manvolnum>5</manvolnum></citerefentry>.
@@ -1253,6 +1258,74 @@ ExecStart=/bin/echo $ONE $TWO $THREE</programlisting>
                 <literal>>/dev/null</literal>,
                 <literal>&</literal>, <literal>;</literal>, and
                 <literal>/bin/ls</literal>.</para>
+
+                <table>
+                        <title>C escapes supported in command lines and environment variables</title>
+                        <tgroup cols='2'>
+                                <colspec colname='escape' />
+                                <colspec colname='meaning' />
+                                <thead>
+                                        <row>
+                                                <entry>Literal</entry>
+                                                <entry>Actual value</entry>
+                                        </row>
+                                </thead>
+                                <tbody>
+                                        <row>
+                                                <entry><literal>\a</literal></entry>
+                                                <entry>bell</entry>
+                                        </row>
+                                        <row>
+                                                <entry><literal>\b</literal></entry>
+                                                <entry>backspace</entry>
+                                        </row>
+                                        <row>
+                                                <entry><literal>\f</literal></entry>
+                                                <entry>form feed</entry>
+                                        </row>
+                                        <row>
+                                                <entry><literal>\n</literal></entry>
+                                                <entry>newline</entry>
+                                        </row>
+                                        <row>
+                                                <entry><literal>\r</literal></entry>
+                                                <entry>carriage return</entry>
+                                        </row>
+                                        <row>
+                                                <entry><literal>\t</literal></entry>
+                                                <entry>tab</entry>
+                                        </row>
+                                        <row>
+                                                <entry><literal>\v</literal></entry>
+                                                <entry>vertical tab</entry>
+                                        </row>
+                                        <row>
+                                                <entry><literal>\\</literal></entry>
+                                                <entry>backslash</entry>
+                                        </row>
+                                        <row>
+                                                <entry><literal>\"</literal></entry>
+                                                <entry>double quotation mark</entry>
+                                        </row>
+                                        <row>
+                                                <entry><literal>\'</literal></entry>
+                                                <entry>single quotation mark</entry>
+                                        </row>
+                                        <row>
+                                                <entry><literal>\s</literal></entry>
+                                                <entry>space</entry>
+                                        </row>
+                                        <row>
+                                                <entry><literal>\x<replaceable>xx</replaceable></literal></entry>
+                                                <entry>character number <replaceable>xx</replaceable> in hexadecimal encoding</entry>
+                                        </row>
+                                        <row>
+                                                <entry><literal>\<replaceable>nnn</replaceable></literal></entry>
+                                                <entry>character number <replaceable>nnn</replaceable> in octal encoding</entry>
+                                        </row>
+                                </tbody>
+                        </tgroup>
+                </table>
         </refsect1>
 
         <refsect1>
diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c
index 7430036..e9659ca 100644
--- a/src/core/load-fragment.c
+++ b/src/core/load-fragment.c
@@ -547,9 +547,9 @@ int config_parse_exec(const char *unit,
          * overriding of argv[0]. */
         for (;;) {
                 int i;
-                const char *word, *state;
+                const char *word, *state, *reason;
                 size_t l;
-                bool honour_argv0 = false, ignore = false;
+                bool separate_argv0 = false, ignore = false;
 
                 path = NULL;
                 nce = NULL;
@@ -560,28 +560,23 @@ int config_parse_exec(const char *unit,
                 if (rvalue[0] == 0)
                         break;
 
-                for (i = 0; i < 2; i++) {
-                        if (rvalue[0] == '-' && !ignore) {
-                                ignore = true;
-                                rvalue ++;
-                        }
-
-                        if (rvalue[0] == '@' && !honour_argv0) {
-                                honour_argv0 = true;
-                                rvalue ++;
-                        }
-                }
-
-                if (*rvalue != '/') {
-                        log_syntax(unit, LOG_ERR, filename, line, EINVAL,
-                                   "Executable path is not absolute, ignoring: %s", rvalue);
-                        return 0;
-                }
-
                 k = 0;
                 FOREACH_WORD_QUOTED(word, l, rvalue, state) {
-                        if (strneq(word, ";", MAX(l, 1U)))
-                                goto found;
+                        if (k == 0) {
+                                for (i = 0; i < 2; i++) {
+                                        if (*word == '-' && !ignore) {
+                                                ignore = true;
+                                                word ++;
+                                        }
+
+                                        if (*word == '@' && !separate_argv0) {
+                                                separate_argv0 = true;
+                                                word ++;
+                                        }
+                                }
+                        } else
+                                if (strneq(word, ";", MAX(l, 1U)))
+                                        goto found;
 
                         k++;
                 }
@@ -592,60 +587,69 @@ int config_parse_exec(const char *unit,
                 }
 
         found:
-                n = new(char*, k + !honour_argv0);
+                n = new(char*, k + !separate_argv0);
                 if (!n)
                         return log_oom();
 
                 k = 0;
                 FOREACH_WORD_QUOTED(word, l, rvalue, state) {
-                        if (strneq(word, ";", MAX(l, 1U)))
-                                break;
-                        else if (strneq(word, "\\;", MAX(l, 1U))) {
-                                word ++;
-                                l --;
-                        }
+                        char *c;
+                        unsigned skip;
 
-                        if (honour_argv0 && word == rvalue) {
-                                assert(!path);
+                        if (separate_argv0 ? path == NULL : k == 0) {
+                                /* first word, very special */
+                                skip = separate_argv0 + ignore;
 
-                                path = strndup(word, l);
-                                if (!path) {
-                                        r = log_oom();
-                                        goto fail;
-                                }
+                                /* skip special chars in the beginning */
+                                assert(skip < l);
 
-                                if (!utf8_is_valid(path)) {
-                                        log_invalid_utf8(unit, LOG_ERR, filename, line, EINVAL, rvalue);
-                                        r = 0;
-                                        goto fail;
-                                }
+                        } else if (strneq(word, ";", MAX(l, 1U)))
+                                /* new commandline */
+                                break;
 
-                        } else {
-                                char *c;
+                        else
+                                skip = strneq(word, "\\;", MAX(l, 1U));
 
-                                c = n[k++] = cunescape_length(word, l);
-                                if (!c) {
-                                        r = log_oom();
-                                        goto fail;
-                                }
+                        c = cunescape_length(word + skip, l - skip);
+                        if (!c) {
+                                r = log_oom();
+                                goto fail;
+                        }
 
-                                if (!utf8_is_valid(c)) {
-                                        log_invalid_utf8(unit, LOG_ERR, filename, line, EINVAL, rvalue);
-                                        r = 0;
-                                        goto fail;
-                                }
+                        if (!utf8_is_valid(c)) {
+                                log_invalid_utf8(unit, LOG_ERR, filename, line, EINVAL, rvalue);
+                                r = 0;
+                                goto fail;
                         }
+
+                        /* where to stuff this? */
+                        if (separate_argv0 && path == NULL)
+                                path = c;
+                        else
+                                n[k++] = c;
                 }
 
                 n[k] = NULL;
 
-                if (!n[0]) {
-                        log_syntax(unit, LOG_ERR, filename, line, EINVAL,
-                                   "Invalid command line, ignoring: %s", rvalue);
-                        r = 0;
-                        goto fail;
-                }
+                log_debug("path: %s", path ?: n[0]);
+
+                if (!n[0])
+                        reason = "Empty executable name or zeroeth argument";
+                else if (!string_is_safe(path ?: n[0]))
+                        reason = "Executable path contains special characters";
+                else if (!path_is_absolute(path ?: n[0]))
+                        reason = "Executable path is not absolute";
+                else if (endswith(path ?: n[0], "/"))
+                        reason = "Executable path specifies a directory";
+                else
+                        goto ok;
+
+                log_syntax(unit, LOG_ERR, filename, line, EINVAL,
+                           "%s, ignoring: %s", reason, rvalue);
+                r = 0;
+                goto fail;
 
+ok:
                 if (!path) {
                         path = strdup(n[0]);
                         if (!path) {
@@ -654,8 +658,6 @@ int config_parse_exec(const char *unit,
                         }
                 }
 
-                assert(path_is_absolute(path));
-
                 nce = new0(ExecCommand, 1);
                 if (!nce) {
                         r = log_oom();
diff --git a/src/test/test-unit-file.c b/src/test/test-unit-file.c
index 08da2ba..6a146a7 100644
--- a/src/test/test-unit-file.c
+++ b/src/test/test-unit-file.c
@@ -74,31 +74,34 @@ static void check_execcommand(ExecCommand *c,
                               const char* argv2,
                               bool ignore) {
         assert_se(c);
-        log_info("%s %s %s %s",
+        log_info("expect: \"%s\" [\"%s\" \"%s\" \"%s\"]",
+                 path, argv0 ?: path, argv1, argv2);
+        log_info("actual: \"%s\" [\"%s\" \"%s\" \"%s\"]",
                  c->path, c->argv[0], c->argv[1], c->argv[2]);
         assert_se(streq(c->path, path));
-        assert_se(streq(c->argv[0], argv0));
-        assert_se(streq(c->argv[1], argv1));
+        assert_se(streq(c->argv[0], argv0 ?: path));
+        assert_se(streq_ptr(c->argv[1], argv1));
         assert_se(streq_ptr(c->argv[2], argv2));
         assert_se(c->ignore == ignore);
 }
 
 static void test_config_parse_exec(void) {
-        /* int config_parse_exec( */
-        /*         const char *filename, */
-        /*         unsigned line, */
-        /*         const char *section, */
-        /*         unsigned section_line, */
-        /*         const char *lvalue, */
-        /*         int ltype, */
-        /*         const char *rvalue, */
-        /*         void *data, */
-        /*         void *userdata) */
+        /* int config_parse_exec(
+                 const char *filename,
+                 unsigned line,
+                 const char *section,
+                 unsigned section_line,
+                 const char *lvalue,
+                 int ltype,
+                 const char *rvalue,
+                 void *data,
+                 void *userdata) */
         int r;
 
         ExecCommand *c = NULL, *c1;
+        const char *ccc;
 
-        /* basic test */
+        log_info("/* basic test */");
         r = config_parse_exec(NULL, "fake", 1, "section", 1,
                               "LValue", 0, "/RValue r1",
                               &c, NULL);
@@ -106,52 +109,60 @@ static void test_config_parse_exec(void) {
         check_execcommand(c, "/RValue", "/RValue", "r1", NULL, false);
 
         r = config_parse_exec(NULL, "fake", 2, "section", 1,
-                              "LValue", 0, "/RValue///slashes/// r1",
+                              "LValue", 0, "/RValue///slashes r1///",
                               &c, NULL);
-       /* test slashes */
+
+        log_info("/* test slashes */");
         assert_se(r >= 0);
         c1 = c->command_next;
-        check_execcommand(c1, "/RValue/slashes", "/RValue///slashes///", "r1", NULL, false);
+        check_execcommand(c1, "/RValue/slashes", "/RValue///slashes", "r1///", NULL, false);
 
-        /* honour_argv0 */
+        log_info("/* trailing slash */");
+        r = config_parse_exec(NULL, "fake", 4, "section", 1,
+                              "LValue", 0, "/RValue/ argv0 r1",
+                              &c, NULL);
+        assert_se(r == 0);
+        assert_se(c1->command_next == NULL);
+
+        log_info("/* honour_argv0 */");
         r = config_parse_exec(NULL, "fake", 3, "section", 1,
-                              "LValue", 0, "@/RValue///slashes2/// argv0 r1",
+                              "LValue", 0, "@/RValue///slashes2 ///argv0 r1",
                               &c, NULL);
         assert_se(r >= 0);
         c1 = c1->command_next;
-        check_execcommand(c1, "/RValue/slashes2", "argv0", "r1", NULL, false);
+        check_execcommand(c1, "/RValue/slashes2", "///argv0", "r1", NULL, false);
 
-        /* ignore && honour_argv0 */
+        log_info("/* ignore && honour_argv0 */");
         r = config_parse_exec(NULL, "fake", 4, "section", 1,
-                              "LValue", 0, "-@/RValue///slashes3/// argv0a r1",
+                              "LValue", 0, "-@/RValue///slashes3 argv0a r1",
                               &c, NULL);
         assert_se(r >= 0);
         c1 = c1->command_next;
         check_execcommand(c1, "/RValue/slashes3", "argv0a", "r1", NULL, true);
 
-        /* ignore && honour_argv0 */
+        log_info("/* ignore && honour_argv0 */");
         r = config_parse_exec(NULL, "fake", 4, "section", 1,
-                              "LValue", 0, "@-/RValue///slashes4/// argv0b r1",
+                              "LValue", 0, "@-/RValue///slashes4 argv0b r1",
                               &c, NULL);
         assert_se(r >= 0);
         c1 = c1->command_next;
         check_execcommand(c1, "/RValue/slashes4", "argv0b", "r1", NULL, true);
 
-        /* ignore && ignore */
+        log_info("/* ignore && ignore */");
         r = config_parse_exec(NULL, "fake", 4, "section", 1,
                               "LValue", 0, "--/RValue argv0 r1",
                               &c, NULL);
         assert_se(r == 0);
         assert_se(c1->command_next == NULL);
 
-        /* ignore && ignore */
+        log_info("/* ignore && ignore (2) */");
         r = config_parse_exec(NULL, "fake", 4, "section", 1,
                               "LValue", 0, "- at -/RValue argv0 r1",
                               &c, NULL);
         assert_se(r == 0);
         assert_se(c1->command_next == NULL);
 
-        /* semicolon */
+        log_info("/* semicolon */");
         r = config_parse_exec(NULL, "fake", 5, "section", 1,
                               "LValue", 0,
                               "-@/RValue argv0 r1 ; "
@@ -162,9 +173,9 @@ static void test_config_parse_exec(void) {
         check_execcommand(c1, "/RValue", "argv0", "r1", NULL, true);
 
         c1 = c1->command_next;
-        check_execcommand(c1, "/goo/goo", "/goo/goo", "boo", NULL, false);
+        check_execcommand(c1, "/goo/goo", NULL, "boo", NULL, false);
 
-        /* trailing semicolon */
+        log_info("/* trailing semicolon */");
         r = config_parse_exec(NULL, "fake", 5, "section", 1,
                               "LValue", 0,
                               "-@/RValue argv0 r1 ; ",
@@ -175,16 +186,16 @@ static void test_config_parse_exec(void) {
 
         assert_se(c1->command_next == NULL);
 
-        /* escaped semicolon */
+        log_info("/* escaped semicolon */");
         r = config_parse_exec(NULL, "fake", 5, "section", 1,
                               "LValue", 0,
                               "/bin/find \\;",
                               &c, NULL);
         assert_se(r >= 0);
         c1 = c1->command_next;
-        check_execcommand(c1, "/bin/find", "/bin/find", ";", NULL, false);
+        check_execcommand(c1, "/bin/find", NULL, ";", NULL, false);
 
-        /* escaped semicolon with following arg */
+        log_info("/* escaped semicolon with following arg */");
         r = config_parse_exec(NULL, "fake", 5, "section", 1,
                               "LValue", 0,
                               "/sbin/find \\; x",
@@ -192,7 +203,86 @@ static void test_config_parse_exec(void) {
         assert_se(r >= 0);
         c1 = c1->command_next;
         check_execcommand(c1,
-                          "/sbin/find", "/sbin/find", ";", "x", false);
+                          "/sbin/find", NULL, ";", "x", false);
+
+        log_info("/* spaces in the filename */");
+        r = config_parse_exec(NULL, "fake", 5, "section", 1,
+                              "LValue", 0,
+                              "\"/PATH WITH SPACES/daemon\" -1 -2",
+                              &c, NULL);
+        assert_se(r >= 0);
+        c1 = c1->command_next;
+        check_execcommand(c1,
+                          "/PATH WITH SPACES/daemon", NULL, "-1", "-2", false);
+
+        log_info("/* spaces in the filename, no args */");
+        r = config_parse_exec(NULL, "fake", 5, "section", 1,
+                              "LValue", 0,
+                              "\"/PATH WITH SPACES/daemon -1 -2\"",
+                              &c, NULL);
+        assert_se(r >= 0);
+        c1 = c1->command_next;
+        check_execcommand(c1,
+                          "/PATH WITH SPACES/daemon -1 -2", NULL, NULL, NULL, false);
+
+        log_info("/* spaces in the filename, everything quoted */");
+        r = config_parse_exec(NULL, "fake", 5, "section", 1,
+                              "LValue", 0,
+                              "\"/PATH WITH SPACES/daemon\" \"-1\" '-2'",
+                              &c, NULL);
+        assert_se(r >= 0);
+        c1 = c1->command_next;
+        check_execcommand(c1,
+                          "/PATH WITH SPACES/daemon", NULL, "-1", "-2", false);
+
+        log_info("/* escaped spaces in the filename */");
+        r = config_parse_exec(NULL, "fake", 5, "section", 1,
+                              "LValue", 0,
+                              "\"/PATH\\sWITH\\sSPACES/daemon\" '-1 -2'",
+                              &c, NULL);
+        assert_se(r >= 0);
+        c1 = c1->command_next;
+        check_execcommand(c1,
+                          "/PATH WITH SPACES/daemon", NULL, "-1 -2", NULL, false);
+
+        log_info("/* escaped spaces in the filename (2) */");
+        r = config_parse_exec(NULL, "fake", 5, "section", 1,
+                              "LValue", 0,
+                              "\"/PATH\\x20WITH\\x20SPACES/daemon\" \"-1 -2\"",
+                              &c, NULL);
+        assert_se(r >= 0);
+        c1 = c1->command_next;
+        check_execcommand(c1,
+                          "/PATH WITH SPACES/daemon", NULL, "-1 -2", NULL, false);
+
+        for (ccc = "abfnrtv\\\'\"x"; *ccc; ccc++) {
+                /* \\x is an incomplete hexadecimal sequence, invalid because of the slash */
+                char path[] = "/path\\X";
+                path[sizeof(path) - 2] = *ccc;
+
+                log_info("/* invalid character: \\%c */", *ccc);
+                r = config_parse_exec(NULL, "fake", 4, "section", 1,
+                                      "LValue", 0, path,
+                                      &c, NULL);
+                assert_se(r == 0);
+                assert_se(c1->command_next == NULL);
+        }
+
+        log_info("/* valid character: \\s */");
+        r = config_parse_exec(NULL, "fake", 4, "section", 1,
+                              "LValue", 0, "/path\\s",
+                              &c, NULL);
+        assert_se(r >= 0);
+        c1 = c1->command_next;
+        check_execcommand(c1, "/path ", NULL, NULL, NULL, false);
+
+        log_info("/* trailing backslash: \\ */");
+        /* backslash is invalid */
+        r = config_parse_exec(NULL, "fake", 4, "section", 1,
+                              "LValue", 0, "/path\\",
+                              &c, NULL);
+        assert_se(r == 0);
+        assert_se(c1->command_next == NULL);
 
         exec_command_free_list(c);
 }

commit ba774317ac7d3e67fdb9ed81663264d38859df59
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Thu Dec 18 17:51:38 2014 -0500

    Treat a trailing backslash as an error
    
    Commit a2a5291b3f5 changed the parser to reject unfinished quoted
    strings. Unfortunately it introduced an error where a trailing
    backslash would case an infinite loop. Of course this must fixed, but
    the question is what to to instead. Allowing trailing backslashes and
    treating them as normal characters would be one option, but this seems
    suboptimal. First, there would be inconsistency between handling of
    quoting and of backslashes. Second, a trailing backslash is most
    likely an error, at it seems better to point it out to the user than
    to try to continue.
    
    Updated rules:
    ExecStart=/bin/echo \\ → OK, prints a backslash
    ExecStart=/bin/echo \ → error
    ExecStart=/bin/echo "x → error
    ExecStart=/bin/echo "x"y → error

diff --git a/src/shared/util.c b/src/shared/util.c
index 364f618..91cf670 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -521,7 +521,7 @@ int safe_atod(const char *s, double *ret_d) {
 
 static size_t strcspn_escaped(const char *s, const char *reject) {
         bool escaped = false;
-        size_t n;
+        int n;
 
         for (n=0; s[n]; n++) {
                 if (escaped)
@@ -531,6 +531,7 @@ static size_t strcspn_escaped(const char *s, const char *reject) {
                 else if (strchr(reject, s[n]))
                         break;
         }
+
         /* if s ends in \, return index of previous char */
         return n - escaped;
 }
@@ -566,6 +567,11 @@ const char* split(const char **state, size_t *l, const char *separator, bool quo
                 *state = current++ + *l + 2;
         } else if (quoted) {
                 *l = strcspn_escaped(current, separator);
+                if (current[*l] && !strchr(separator, current[*l])) {
+                        /* unfinished escape */
+                        *state = current;
+                        return NULL;
+                }
                 *state = current + *l;
         } else {
                 *l = strcspn(current, separator);
diff --git a/src/test/test-strv.c b/src/test/test-strv.c
index 0b78086..f343eab 100644
--- a/src/test/test-strv.c
+++ b/src/test/test-strv.c
@@ -520,6 +520,10 @@ int main(int argc, char *argv[]) {
         test_strv_unquote("  \"x'\"   ", STRV_MAKE("x'"));
         test_strv_unquote("a  '--b=c \"d e\"'", STRV_MAKE("a", "--b=c \"d e\""));
 
+        /* trailing backslashes */
+        test_strv_unquote("  x\\\\", STRV_MAKE("x\\"));
+        test_invalid_unquote("  x\\");
+
         test_invalid_unquote("a  --b='c \"d e\"''");
         test_invalid_unquote("a  --b='c \"d e\" '\"");
         test_invalid_unquote("a  --b='c \"d e\"garbage");
diff --git a/src/test/test-util.c b/src/test/test-util.c
index bbf7512..222af9a 100644
--- a/src/test/test-util.c
+++ b/src/test/test-util.c
@@ -406,28 +406,12 @@ static void test_foreach_word(void) {
                 assert_se(strneq(expected[i++], word, l));
 }
 
-static void test_foreach_word_quoted(void) {
+static void check(const char *test, char** expected, bool trailing) {
         const char *word, *state;
         size_t l;
         int i = 0;
-        const char test[] = "test a b c 'd' e '' '' hhh '' '' \"a b c\"";
-        const char * const expected[] = {
-                "test",
-                "a",
-                "b",
-                "c",
-                "d",
-                "e",
-                "",
-                "",
-                "hhh",
-                "",
-                "",
-                "a b c",
-                NULL
-        };
 
-        printf("<%s>\n", test);
+        printf("<<<%s>>>\n", test);
         FOREACH_WORD_QUOTED(word, l, test, state) {
                 _cleanup_free_ char *t = NULL;
 
@@ -435,7 +419,34 @@ static void test_foreach_word_quoted(void) {
                 assert_se(strneq(expected[i++], word, l));
                 printf("<%s>\n", t);
         }
-        assert_se(isempty(state));
+        printf("<<<%s>>>\n", state);
+        assert(expected[i] == NULL);
+        assert_se(isempty(state) == !trailing);
+}
+
+static void test_foreach_word_quoted(void) {
+        check("test a b c 'd' e '' '' hhh '' '' \"a b c\"",
+              STRV_MAKE("test",
+                        "a",
+                        "b",
+                        "c",
+                        "d",
+                        "e",
+                        "",
+                        "",
+                        "hhh",
+                        "",
+                        "",
+                        "a b c"),
+              false);
+
+        check("test \"xxx",
+              STRV_MAKE("test"),
+              true);
+
+        check("test\\",
+              STRV_MAKE_EMPTY,
+              true);
 }
 
 static void test_default_term_for_tty(void) {

commit 30bcc05295944cfc2f3ed9159592130c003e19f5
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Thu Dec 18 17:01:42 2014 -0500

    test-strv: use STRV_MAKE

diff --git a/src/test/test-strv.c b/src/test/test-strv.c
index e09b4fb..0b78086 100644
--- a/src/test/test-strv.c
+++ b/src/test/test-strv.c
@@ -175,7 +175,7 @@ static void test_strv_quote_unquote(const char* const *split, const char *quoted
         }
 }
 
-static void test_strv_unquote(const char *quoted, const char **list) {
+static void test_strv_unquote(const char *quoted, char **list) {
         _cleanup_strv_free_ char **s;
         _cleanup_free_ char *j;
         unsigned i = 0;
@@ -507,18 +507,18 @@ int main(int argc, char *argv[]) {
         test_strv_quote_unquote(input_table_quotes, QUOTES_STRING);
         test_strv_quote_unquote(input_table_spaces, SPACES_STRING);
 
-        test_strv_unquote("    foo=bar     \"waldo\"    zzz    ", (const char*[]) { "foo=bar", "waldo", "zzz", NULL });
-        test_strv_unquote("", (const char*[]) { NULL });
-        test_strv_unquote(" ", (const char*[]) { NULL });
-        test_strv_unquote("   ", (const char*[]) { NULL });
-        test_strv_unquote("   x", (const char*[]) { "x", NULL });
-        test_strv_unquote("x   ", (const char*[]) { "x", NULL });
-        test_strv_unquote("  x   ", (const char*[]) { "x", NULL });
-        test_strv_unquote("  \"x\"   ", (const char*[]) { "x", NULL });
-        test_strv_unquote("  'x'   ", (const char*[]) { "x", NULL });
-        test_strv_unquote("  'x\"'   ", (const char*[]) { "x\"", NULL });
-        test_strv_unquote("  \"x'\"   ", (const char*[]) { "x'", NULL });
-        test_strv_unquote("a  '--b=c \"d e\"'", (const char*[]) { "a", "--b=c \"d e\"", NULL });
+        test_strv_unquote("    foo=bar     \"waldo\"    zzz    ", STRV_MAKE("foo=bar", "waldo", "zzz"));
+        test_strv_unquote("", STRV_MAKE_EMPTY);
+        test_strv_unquote(" ", STRV_MAKE_EMPTY);
+        test_strv_unquote("   ", STRV_MAKE_EMPTY);
+        test_strv_unquote("   x", STRV_MAKE("x"));
+        test_strv_unquote("x   ", STRV_MAKE("x"));
+        test_strv_unquote("  x   ", STRV_MAKE("x"));
+        test_strv_unquote("  \"x\"   ", STRV_MAKE("x"));
+        test_strv_unquote("  'x'   ", STRV_MAKE("x"));
+        test_strv_unquote("  'x\"'   ", STRV_MAKE("x\""));
+        test_strv_unquote("  \"x'\"   ", STRV_MAKE("x'"));
+        test_strv_unquote("a  '--b=c \"d e\"'", STRV_MAKE("a", "--b=c \"d e\""));
 
         test_invalid_unquote("a  --b='c \"d e\"''");
         test_invalid_unquote("a  --b='c \"d e\" '\"");

commit 447021aafdf4bd0c0f70a3078d35a8f00ada4504
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Thu Dec 18 12:33:05 2014 -0500

    tree-wide: make condition_free_list return NULL

diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c
index e8dfa1a..7430036 100644
--- a/src/core/load-fragment.c
+++ b/src/core/load-fragment.c
@@ -2042,8 +2042,7 @@ int config_parse_unit_condition_path(
 
         if (isempty(rvalue)) {
                 /* Empty assignment resets the list */
-                condition_free_list(*list);
-                *list = NULL;
+                *list = condition_free_list(*list);
                 return 0;
         }
 
@@ -2100,8 +2099,7 @@ int config_parse_unit_condition_string(
 
         if (isempty(rvalue)) {
                 /* Empty assignment resets the list */
-                condition_free_list(*list);
-                *list = NULL;
+                *list = condition_free_list(*list);
                 return 0;
         }
 
@@ -2150,8 +2148,7 @@ int config_parse_unit_condition_null(
 
         if (isempty(rvalue)) {
                 /* Empty assignment resets the list */
-                condition_free_list(*list);
-                *list = NULL;
+                *list = condition_free_list(*list);
                 return 0;
         }
 
diff --git a/src/shared/condition.c b/src/shared/condition.c
index dcbf9a7..3a34529 100644
--- a/src/shared/condition.c
+++ b/src/shared/condition.c
@@ -73,11 +73,13 @@ void condition_free(Condition *c) {
         free(c);
 }
 
-void condition_free_list(Condition *first) {
+Condition* condition_free_list(Condition *first) {
         Condition *c, *n;
 
         LIST_FOREACH_SAFE(conditions, c, n, first)
                 condition_free(c);
+
+        return NULL;
 }
 
 static int condition_test_kernel_command_line(Condition *c) {
diff --git a/src/shared/condition.h b/src/shared/condition.h
index 28d1d94..0780e78 100644
--- a/src/shared/condition.h
+++ b/src/shared/condition.h
@@ -79,7 +79,7 @@ typedef struct Condition {
 
 Condition* condition_new(ConditionType type, const char *parameter, bool trigger, bool negate);
 void condition_free(Condition *c);
-void condition_free_list(Condition *c);
+Condition* condition_free_list(Condition *c);
 
 int condition_test(Condition *c);
 

commit f1acf85a36f4c32d69511fe1bfa12f66e28fa80d
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Thu Dec 18 12:29:24 2014 -0500

    core: make exec_command_free_list return NULL

diff --git a/src/core/dbus-service.c b/src/core/dbus-service.c
index 5a881e8..2b50ac9 100644
--- a/src/core/dbus-service.c
+++ b/src/core/dbus-service.c
@@ -187,10 +187,8 @@ static int bus_service_set_transient_property(
                         ExecCommand *c;
                         size_t size = 0;
 
-                        if (n == 0) {
-                                exec_command_free_list(s->exec_command[SERVICE_EXEC_START]);
-                                s->exec_command[SERVICE_EXEC_START] = NULL;
-                        }
+                        if (n == 0)
+                                s->exec_command[SERVICE_EXEC_START] = exec_command_free_list(s->exec_command[SERVICE_EXEC_START]);
 
                         f = open_memstream(&buf, &size);
                         if (!f)
diff --git a/src/core/execute.c b/src/core/execute.c
index ae2a52d..bc925cd 100644
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -2007,7 +2007,7 @@ void exec_command_done_array(ExecCommand *c, unsigned n) {
                 exec_command_done(c+i);
 }
 
-void exec_command_free_list(ExecCommand *c) {
+ExecCommand* exec_command_free_list(ExecCommand *c) {
         ExecCommand *i;
 
         while ((i = c)) {
@@ -2015,15 +2015,15 @@ void exec_command_free_list(ExecCommand *c) {
                 exec_command_done(i);
                 free(i);
         }
+
+        return NULL;
 }
 
 void exec_command_free_array(ExecCommand **c, unsigned n) {
         unsigned i;
 
-        for (i = 0; i < n; i++) {
-                exec_command_free_list(c[i]);
-                c[i] = NULL;
-        }
+        for (i = 0; i < n; i++)
+                c[i] = exec_command_free_list(c[i]);
 }
 
 int exec_context_load_environment(const ExecContext *c, const char *unit_id, char ***l) {
diff --git a/src/core/execute.h b/src/core/execute.h
index 5ed7505..2c20139 100644
--- a/src/core/execute.h
+++ b/src/core/execute.h
@@ -228,7 +228,7 @@ int exec_spawn(ExecCommand *command,
 void exec_command_done(ExecCommand *c);
 void exec_command_done_array(ExecCommand *c, unsigned n);
 
-void exec_command_free_list(ExecCommand *c);
+ExecCommand* exec_command_free_list(ExecCommand *c);
 void exec_command_free_array(ExecCommand **c, unsigned n);
 
 char *exec_command_line(char **argv);
diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c
index 358d36b..e8dfa1a 100644
--- a/src/core/load-fragment.c
+++ b/src/core/load-fragment.c
@@ -538,8 +538,7 @@ int config_parse_exec(const char *unit,
 
         if (isempty(rvalue)) {
                 /* An empty assignment resets the list */
-                exec_command_free_list(*e);
-                *e = NULL;
+                *e = exec_command_free_list(*e);
                 return 0;
         }
 



More information about the systemd-commits mailing list