Flagsify EscapeStyle and make ESCAPE_BACKSLASH_ONELINE implicit

I want to tweak behaviour further, and that'll be easier when "style"
is converted to a bitfield.

Some callers used ESCAPE_BACKSLASH_ONELINE, and others not. But the
ones that didn't, simply didn't care, because the argument was assumed to
be one-line anyway (e.g. a service name). In environment-generator, this
could make a difference. But I think it's better to escape the newlines
there too. So newlines are now always escaped, to simplify the code and
the test matrix.

diff --git a/src/basic/escape.c b/src/basic/escape.c
index af785ecfa457752879845f65e7ae6fa2f8fc2029..7a9a3b56166e905598292835a0b4711ae158aca5 100644 (file)
--- a/src/basic/escape.c
+++ b/src/basic/escape.c
@@ -494,7 +494,7 @@ char* shell_escape(const char *s, const char *bad) {
         return r;
 }
 
-char* shell_maybe_quote(const char *s, EscapeStyle style) {
+char* shell_maybe_quote(const char *s, ShellEscapeFlags flags) {
         const char *p;
         char *r, *t;
 
@@ -513,36 +513,27 @@ char* shell_maybe_quote(const char *s, EscapeStyle style) {
         if (!*p)
                 return strdup(s);
 
-        r = new(char, (style == ESCAPE_POSIX) + 1 + strlen(s)*2 + 1 + 1);
+        r = new(char, FLAGS_SET(flags, SHELL_ESCAPE_POSIX) + 1 + strlen(s)*2 + 1 + 1);
         if (!r)
                 return NULL;
 
         t = r;
-        switch (style) {
-        case ESCAPE_BACKSLASH:
-        case ESCAPE_BACKSLASH_ONELINE:
-                *(t++) = '"';
-                break;
-        case ESCAPE_POSIX:
+        if (FLAGS_SET(flags, SHELL_ESCAPE_POSIX)) {
                 *(t++) = '$';
                 *(t++) = '\'';
-                break;
-        default:
-                assert_not_reached("Bad EscapeStyle");
-        }
+        } else
+                *(t++) = '"';
 
         t = mempcpy(t, s, p - s);
 
-        if (IN_SET(style, ESCAPE_BACKSLASH, ESCAPE_BACKSLASH_ONELINE))
-                t = strcpy_backslash_escaped(t, p, SHELL_NEED_ESCAPE,
-                                             style == ESCAPE_BACKSLASH_ONELINE);
-        else
-                t = strcpy_backslash_escaped(t, p, SHELL_NEED_ESCAPE_POSIX, true);
+        t = strcpy_backslash_escaped(t, p,
+                                     FLAGS_SET(flags, SHELL_ESCAPE_POSIX) ? SHELL_NEED_ESCAPE_POSIX : SHELL_NEED_ESCAPE,
+                                     true);
 
-        if (IN_SET(style, ESCAPE_BACKSLASH, ESCAPE_BACKSLASH_ONELINE))
-                *(t++) = '"';
-        else
+        if (FLAGS_SET(flags, SHELL_ESCAPE_POSIX))
                 *(t++) = '\'';
+        else
+                *(t++) = '"';
         *t = 0;
 
         return r;

diff --git a/src/basic/escape.h b/src/basic/escape.h
index 691b6d802c296620f56a51dce15e004af93677f0..7ecae49e10d5a86228af4d6a2817e4c2072428e1 100644 (file)
--- a/src/basic/escape.h
+++ b/src/basic/escape.h
@@ -33,15 +33,12 @@ typedef enum UnescapeFlags {
         UNESCAPE_ACCEPT_NUL = 1 << 1,
 } UnescapeFlags;
 
-typedef enum EscapeStyle {
-        ESCAPE_BACKSLASH         = 1,  /* Add shell quotes ("") so the shell will consider this a single
-                                          argument, possibly multiline. Tabs and newlines are not escaped. */
-        ESCAPE_BACKSLASH_ONELINE = 2,  /* Similar to ESCAPE_BACKSLASH, but always produces a single-line
-                                          string instead. Shell escape sequences are produced for tabs and
-                                          newlines. */
-        ESCAPE_POSIX             = 3,  /* Similar to ESCAPE_BACKSLASH_ONELINE, but uses POSIX shell escape
-                                        * syntax (a string enclosed in $'') instead of plain quotes. */
-} EscapeStyle;
+typedef enum ShellEscapeFlags {
+        /* The default is to add shell quotes ("") so the shell will consider this a single argument.
+         * Tabs and newlines are escaped. */
+
+        SHELL_ESCAPE_POSIX = 1 << 1, /* Use POSIX shell escape syntax (a string enclosed in $'') instead of plain quotes. */
+} ShellEscapeFlags;
 
 char* cescape(const char *s);
 char* cescape_length(const char *s, size_t n);
@@ -64,4 +61,4 @@ char* octescape(const char *s, size_t len);
 char* escape_non_printable_full(const char *str, size_t console_width, bool eight_bit);
 
 char* shell_escape(const char *s, const char *bad);
-char* shell_maybe_quote(const char *s, EscapeStyle style);
+char* shell_maybe_quote(const char *s, ShellEscapeFlags flags);

diff --git a/src/core/job.c b/src/core/job.c
index 56c99f93eb1cee00455060d27e8aa8d723068be7..639dc352a6c0db44080d8a9d9cc1f9a708f830e5 100644 (file)
--- a/src/core/job.c
+++ b/src/core/job.c
@@ -846,7 +846,7 @@ static void job_print_done_status_message(Unit *u, JobType t, JobResult result)
         if (t == JOB_START && result == JOB_FAILED) {
                 _cleanup_free_ char *quoted;
 
-                quoted = shell_maybe_quote(u->id, ESCAPE_BACKSLASH);
+                quoted = shell_maybe_quote(u->id, 0);
                 manager_status_printf(u->manager, STATUS_TYPE_NORMAL, NULL, "See 'systemctl status %s' for details.", strna(quoted));
         }
 }

diff --git a/src/environment-d-generator/environment-d-generator.c b/src/environment-d-generator/environment-d-generator.c
index 1c51cf6b2cf7eb06d4a03e9e59e6459ec8ca3d09..5372f6cec28bbd21c3ea522241a163b31e08e1a2 100644 (file)
--- a/src/environment-d-generator/environment-d-generator.c
+++ b/src/environment-d-generator/environment-d-generator.c
@@ -70,7 +70,7 @@ static int load_and_print(void) {
                 t = strchr(*i, '=');
                 assert(t);
 
-                q = shell_maybe_quote(t + 1, ESCAPE_BACKSLASH);
+                q = shell_maybe_quote(t + 1, 0);
                 if (!q)
                         return log_oom();
 

diff --git a/src/shared/bus-print-properties.c b/src/shared/bus-print-properties.c
index 2a712be7339fc936a4d1926f9b9badce55ccc9eb..dc16c2f763582a246a73148c1337217428d0909e 100644 (file)
--- a/src/shared/bus-print-properties.c
+++ b/src/shared/bus-print-properties.c
@@ -270,7 +270,7 @@ static int bus_print_property(const char *name, const char *expected_value, sd_b
                         while ((r = sd_bus_message_read_basic(m, SD_BUS_TYPE_STRING, &str)) > 0) {
                                 _cleanup_free_ char *e = NULL;
 
-                                e = shell_maybe_quote(str, ESCAPE_BACKSLASH_ONELINE);
+                                e = shell_maybe_quote(str, 0);
                                 if (!e)
                                         return -ENOMEM;
 

diff --git a/src/shared/bus-wait-for-jobs.c b/src/shared/bus-wait-for-jobs.c
index 51b71ecc2c18a6758ac2b956896d4c109c2d0a91..f325dd4e6d06ca5ab7240b2f48c7f18fa96b81a2 100644 (file)
--- a/src/shared/bus-wait-for-jobs.c
+++ b/src/shared/bus-wait-for-jobs.c
@@ -181,7 +181,7 @@ static void log_job_error_with_service_result(const char* service, const char *r
 
         assert(service);
 
-        service_shell_quoted = shell_maybe_quote(service, ESCAPE_BACKSLASH);
+        service_shell_quoted = shell_maybe_quote(service, 0);
 
         if (!strv_isempty((char**) extra_args)) {
                 _cleanup_free_ char *t;

diff --git a/src/systemctl/systemctl-set-environment.c b/src/systemctl/systemctl-set-environment.c
index bfd87c0cdd0af2b0c8515977ad4920172fe3ae67..a19e031dd38c509f8f2be46fcf58d4cb4a3a3579 100644 (file)
--- a/src/systemctl/systemctl-set-environment.c
+++ b/src/systemctl/systemctl-set-environment.c
@@ -17,7 +17,7 @@ static int print_variable(const char *s) {
                 return log_error_errno(SYNTHETIC_ERRNO(EUCLEAN),
                                        "Invalid environment block");
 
-        esc = shell_maybe_quote(sep + 1, ESCAPE_POSIX);
+        esc = shell_maybe_quote(sep + 1, SHELL_ESCAPE_POSIX);
         if (!esc)
                 return log_oom();
 

diff --git a/src/test/test-escape.c b/src/test/test-escape.c
index 3e410ca299ed7af3c2ac0a5c2afbf2a95b8d10aa..1e6536884209f8543b5d3370c39e6b84a8bc4403 100644 (file)
--- a/src/test/test-escape.c
+++ b/src/test/test-escape.c
@@ -129,56 +129,43 @@ static void test_shell_escape(void) {
         test_shell_escape_one("foo:bar,baz", ",:", "foo\\:bar\\,baz");
 }
 
-static void test_shell_maybe_quote_one(const char *s,
-                                       EscapeStyle style,
-                                       const char *expected) {
+static void test_shell_maybe_quote_one(const char *s, ShellEscapeFlags flags, const char *expected) {
         _cleanup_free_ char *ret = NULL;
 
-        assert_se(ret = shell_maybe_quote(s, style));
+        assert_se(ret = shell_maybe_quote(s, flags));
         log_debug("[%s] → [%s] (%s)", s, ret, expected);
         assert_se(streq(ret, expected));
 }
 
 static void test_shell_maybe_quote(void) {
 
-        test_shell_maybe_quote_one("", ESCAPE_BACKSLASH, "");
-        test_shell_maybe_quote_one("", ESCAPE_BACKSLASH_ONELINE, "");
-        test_shell_maybe_quote_one("", ESCAPE_POSIX, "");
-        test_shell_maybe_quote_one("\\", ESCAPE_BACKSLASH, "\"\\\\\"");
-        test_shell_maybe_quote_one("\\", ESCAPE_BACKSLASH_ONELINE, "\"\\\\\"");
-        test_shell_maybe_quote_one("\\", ESCAPE_POSIX, "$'\\\\'");
-        test_shell_maybe_quote_one("\"", ESCAPE_BACKSLASH, "\"\\\"\"");
-        test_shell_maybe_quote_one("\"", ESCAPE_BACKSLASH_ONELINE, "\"\\\"\"");
-        test_shell_maybe_quote_one("\"", ESCAPE_POSIX, "$'\"'");
-        test_shell_maybe_quote_one("foobar", ESCAPE_BACKSLASH, "foobar");
-        test_shell_maybe_quote_one("foobar", ESCAPE_BACKSLASH_ONELINE, "foobar");
-        test_shell_maybe_quote_one("foobar", ESCAPE_POSIX, "foobar");
-        test_shell_maybe_quote_one("foo bar", ESCAPE_BACKSLASH, "\"foo bar\"");
-        test_shell_maybe_quote_one("foo bar", ESCAPE_BACKSLASH_ONELINE, "\"foo bar\"");
-        test_shell_maybe_quote_one("foo bar", ESCAPE_POSIX, "$'foo bar'");
-        test_shell_maybe_quote_one("foo\tbar", ESCAPE_BACKSLASH, "\"foo\tbar\"");
-        test_shell_maybe_quote_one("foo\tbar", ESCAPE_BACKSLASH_ONELINE, "\"foo\\tbar\"");
-        test_shell_maybe_quote_one("foo\tbar", ESCAPE_POSIX, "$'foo\\tbar'");
-        test_shell_maybe_quote_one("foo\nbar", ESCAPE_BACKSLASH, "\"foo\nbar\"");
-        test_shell_maybe_quote_one("foo\nbar", ESCAPE_BACKSLASH_ONELINE, "\"foo\\nbar\"");
-        test_shell_maybe_quote_one("foo\nbar", ESCAPE_POSIX, "$'foo\\nbar'");
-        test_shell_maybe_quote_one("foo \"bar\" waldo", ESCAPE_BACKSLASH, "\"foo \\\"bar\\\" waldo\"");
-        test_shell_maybe_quote_one("foo \"bar\" waldo", ESCAPE_BACKSLASH_ONELINE, "\"foo \\\"bar\\\" waldo\"");
-        test_shell_maybe_quote_one("foo \"bar\" waldo", ESCAPE_POSIX, "$'foo \"bar\" waldo'");
-        test_shell_maybe_quote_one("foo$bar", ESCAPE_BACKSLASH, "\"foo\\$bar\"");
-        test_shell_maybe_quote_one("foo$bar", ESCAPE_BACKSLASH_ONELINE, "\"foo\\$bar\"");
-        test_shell_maybe_quote_one("foo$bar", ESCAPE_POSIX, "$'foo$bar'");
+        test_shell_maybe_quote_one("", 0, "");
+        test_shell_maybe_quote_one("", SHELL_ESCAPE_POSIX, "");
+        test_shell_maybe_quote_one("\\", 0, "\"\\\\\"");
+        test_shell_maybe_quote_one("\\", SHELL_ESCAPE_POSIX, "$'\\\\'");
+        test_shell_maybe_quote_one("\"", 0, "\"\\\"\"");
+        test_shell_maybe_quote_one("\"", SHELL_ESCAPE_POSIX, "$'\"'");
+        test_shell_maybe_quote_one("foobar", 0, "foobar");
+        test_shell_maybe_quote_one("foobar", SHELL_ESCAPE_POSIX, "foobar");
+        test_shell_maybe_quote_one("foo bar", 0, "\"foo bar\"");
+        test_shell_maybe_quote_one("foo bar", SHELL_ESCAPE_POSIX, "$'foo bar'");
+        test_shell_maybe_quote_one("foo\tbar", 0, "\"foo\\tbar\"");
+        test_shell_maybe_quote_one("foo\tbar", SHELL_ESCAPE_POSIX, "$'foo\\tbar'");
+        test_shell_maybe_quote_one("foo\nbar", 0, "\"foo\\nbar\"");
+        test_shell_maybe_quote_one("foo\nbar", SHELL_ESCAPE_POSIX, "$'foo\\nbar'");
+        test_shell_maybe_quote_one("foo \"bar\" waldo", 0, "\"foo \\\"bar\\\" waldo\"");
+        test_shell_maybe_quote_one("foo \"bar\" waldo", SHELL_ESCAPE_POSIX, "$'foo \"bar\" waldo'");
+        test_shell_maybe_quote_one("foo$bar", 0, "\"foo\\$bar\"");
+        test_shell_maybe_quote_one("foo$bar", SHELL_ESCAPE_POSIX, "$'foo$bar'");
 
         /* Note that current users disallow control characters, so this "test"
          * is here merely to establish current behaviour. If control characters
          * were allowed, they should be quoted, i.e. \001 should become \\001. */
-        test_shell_maybe_quote_one("a\nb\001", ESCAPE_BACKSLASH, "\"a\nb\001\"");
-        test_shell_maybe_quote_one("a\nb\001", ESCAPE_BACKSLASH_ONELINE, "\"a\\nb\001\"");
-        test_shell_maybe_quote_one("a\nb\001", ESCAPE_POSIX, "$'a\\nb\001'");
+        test_shell_maybe_quote_one("a\nb\001", 0, "\"a\\nb\001\"");
+        test_shell_maybe_quote_one("a\nb\001", SHELL_ESCAPE_POSIX, "$'a\\nb\001'");
 
-        test_shell_maybe_quote_one("foo!bar", ESCAPE_BACKSLASH, "\"foo!bar\"");
-        test_shell_maybe_quote_one("foo!bar", ESCAPE_BACKSLASH_ONELINE, "\"foo!bar\"");
-        test_shell_maybe_quote_one("foo!bar", ESCAPE_POSIX, "$'foo!bar'");
+        test_shell_maybe_quote_one("foo!bar", 0, "\"foo!bar\"");
+        test_shell_maybe_quote_one("foo!bar", SHELL_ESCAPE_POSIX, "$'foo!bar'");
 }
 
 int main(int argc, char *argv[]) {