From 679b0b0a21cbc14cc226c83ebe94ab2844340bc3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Wed, 3 Mar 2021 11:46:11 +0100
Subject: [PATCH 01/21] =?UTF-8?q?test-process-util:=20getpid=5Fcached()=20?=
 =?UTF-8?q?=E2=86=92=200?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This has the same effect and is less verbose.
---
 src/test/test-process-util.c | 70 ++++++++++++++++++------------------
 1 file changed, 35 insertions(+), 35 deletions(-)

diff --git a/src/test/test-process-util.c b/src/test/test-process-util.c
index 5ca654e193..7f0a771ba7 100644
--- a/src/test/test-process-util.c
+++ b/src/test/test-process-util.c
@@ -245,148 +245,148 @@ static void test_get_process_cmdline_harder(void) {
 
         assert_se(prctl(PR_SET_NAME, "testa") >= 0);
 
-        assert_se(get_process_cmdline(getpid_cached(), SIZE_MAX, 0, &line) == -ENOENT);
+        assert_se(get_process_cmdline(0, SIZE_MAX, 0, &line) == -ENOENT);
 
-        assert_se(get_process_cmdline(getpid_cached(), SIZE_MAX, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        assert_se(get_process_cmdline(0, SIZE_MAX, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
         assert_se(streq(line, "[testa]"));
         line = mfree(line);
 
-        assert_se(get_process_cmdline(getpid_cached(), 0, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        assert_se(get_process_cmdline(0, 0, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
         log_info("'%s'", line);
         assert_se(streq(line, ""));
         line = mfree(line);
 
-        assert_se(get_process_cmdline(getpid_cached(), 1, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        assert_se(get_process_cmdline(0, 1, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
         assert_se(streq(line, "…"));
         line = mfree(line);
 
-        assert_se(get_process_cmdline(getpid_cached(), 2, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        assert_se(get_process_cmdline(0, 2, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
         assert_se(streq(line, "[…"));
         line = mfree(line);
 
-        assert_se(get_process_cmdline(getpid_cached(), 3, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        assert_se(get_process_cmdline(0, 3, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
         assert_se(streq(line, "[t…"));
         line = mfree(line);
 
-        assert_se(get_process_cmdline(getpid_cached(), 4, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        assert_se(get_process_cmdline(0, 4, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
         assert_se(streq(line, "[te…"));
         line = mfree(line);
 
-        assert_se(get_process_cmdline(getpid_cached(), 5, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        assert_se(get_process_cmdline(0, 5, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
         assert_se(streq(line, "[tes…"));
         line = mfree(line);
 
-        assert_se(get_process_cmdline(getpid_cached(), 6, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        assert_se(get_process_cmdline(0, 6, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
         assert_se(streq(line, "[test…"));
         line = mfree(line);
 
-        assert_se(get_process_cmdline(getpid_cached(), 7, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        assert_se(get_process_cmdline(0, 7, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
         assert_se(streq(line, "[testa]"));
         line = mfree(line);
 
-        assert_se(get_process_cmdline(getpid_cached(), 8, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        assert_se(get_process_cmdline(0, 8, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
         assert_se(streq(line, "[testa]"));
         line = mfree(line);
 
         assert_se(write(fd, "foo\0bar", 8) == 8);
 
-        assert_se(get_process_cmdline(getpid_cached(), SIZE_MAX, 0, &line) >= 0);
+        assert_se(get_process_cmdline(0, SIZE_MAX, 0, &line) >= 0);
         log_info("'%s'", line);
         assert_se(streq(line, "foo bar"));
         line = mfree(line);
 
-        assert_se(get_process_cmdline(getpid_cached(), SIZE_MAX, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        assert_se(get_process_cmdline(0, SIZE_MAX, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
         assert_se(streq(line, "foo bar"));
         line = mfree(line);
 
         assert_se(write(fd, "quux", 4) == 4);
-        assert_se(get_process_cmdline(getpid_cached(), SIZE_MAX, 0, &line) >= 0);
+        assert_se(get_process_cmdline(0, SIZE_MAX, 0, &line) >= 0);
         log_info("'%s'", line);
         assert_se(streq(line, "foo bar quux"));
         line = mfree(line);
 
-        assert_se(get_process_cmdline(getpid_cached(), SIZE_MAX, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        assert_se(get_process_cmdline(0, SIZE_MAX, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
         assert_se(streq(line, "foo bar quux"));
         line = mfree(line);
 
-        assert_se(get_process_cmdline(getpid_cached(), 1, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        assert_se(get_process_cmdline(0, 1, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
         assert_se(streq(line, "…"));
         line = mfree(line);
 
-        assert_se(get_process_cmdline(getpid_cached(), 2, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        assert_se(get_process_cmdline(0, 2, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
         assert_se(streq(line, "f…"));
         line = mfree(line);
 
-        assert_se(get_process_cmdline(getpid_cached(), 3, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        assert_se(get_process_cmdline(0, 3, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
         assert_se(streq(line, "fo…"));
         line = mfree(line);
 
-        assert_se(get_process_cmdline(getpid_cached(), 4, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        assert_se(get_process_cmdline(0, 4, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
         assert_se(streq(line, "foo…"));
         line = mfree(line);
 
-        assert_se(get_process_cmdline(getpid_cached(), 5, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        assert_se(get_process_cmdline(0, 5, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
         assert_se(streq(line, "foo …"));
         line = mfree(line);
 
-        assert_se(get_process_cmdline(getpid_cached(), 6, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        assert_se(get_process_cmdline(0, 6, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
         assert_se(streq(line, "foo b…"));
         line = mfree(line);
 
-        assert_se(get_process_cmdline(getpid_cached(), 7, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        assert_se(get_process_cmdline(0, 7, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
         assert_se(streq(line, "foo ba…"));
         line = mfree(line);
 
-        assert_se(get_process_cmdline(getpid_cached(), 8, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        assert_se(get_process_cmdline(0, 8, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
         assert_se(streq(line, "foo bar…"));
         line = mfree(line);
 
-        assert_se(get_process_cmdline(getpid_cached(), 9, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        assert_se(get_process_cmdline(0, 9, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
         assert_se(streq(line, "foo bar …"));
         line = mfree(line);
 
-        assert_se(get_process_cmdline(getpid_cached(), 10, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        assert_se(get_process_cmdline(0, 10, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
         assert_se(streq(line, "foo bar q…"));
         line = mfree(line);
 
-        assert_se(get_process_cmdline(getpid_cached(), 11, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        assert_se(get_process_cmdline(0, 11, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
         assert_se(streq(line, "foo bar qu…"));
         line = mfree(line);
 
-        assert_se(get_process_cmdline(getpid_cached(), 12, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        assert_se(get_process_cmdline(0, 12, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
         assert_se(streq(line, "foo bar quux"));
         line = mfree(line);
 
-        assert_se(get_process_cmdline(getpid_cached(), 13, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        assert_se(get_process_cmdline(0, 13, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
         assert_se(streq(line, "foo bar quux"));
         line = mfree(line);
 
-        assert_se(get_process_cmdline(getpid_cached(), 14, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        assert_se(get_process_cmdline(0, 14, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
         assert_se(streq(line, "foo bar quux"));
         line = mfree(line);
 
-        assert_se(get_process_cmdline(getpid_cached(), 1000, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        assert_se(get_process_cmdline(0, 1000, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
         assert_se(streq(line, "foo bar quux"));
         line = mfree(line);
 
         assert_se(ftruncate(fd, 0) >= 0);
         assert_se(prctl(PR_SET_NAME, "aaaa bbbb cccc") >= 0);
 
-        assert_se(get_process_cmdline(getpid_cached(), SIZE_MAX, 0, &line) == -ENOENT);
+        assert_se(get_process_cmdline(0, SIZE_MAX, 0, &line) == -ENOENT);
 
-        assert_se(get_process_cmdline(getpid_cached(), SIZE_MAX, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        assert_se(get_process_cmdline(0, SIZE_MAX, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
         assert_se(streq(line, "[aaaa bbbb cccc]"));
         line = mfree(line);
 
-        assert_se(get_process_cmdline(getpid_cached(), 10, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        assert_se(get_process_cmdline(0, 10, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
         assert_se(streq(line, "[aaaa bbb…"));
         line = mfree(line);
 
-        assert_se(get_process_cmdline(getpid_cached(), 11, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        assert_se(get_process_cmdline(0, 11, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
         assert_se(streq(line, "[aaaa bbbb…"));
         line = mfree(line);
 
-        assert_se(get_process_cmdline(getpid_cached(), 12, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        assert_se(get_process_cmdline(0, 12, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
         assert_se(streq(line, "[aaaa bbbb …"));
         line = mfree(line);
 

From 9e53c10a0f64bc1ab70877c245d5a8e08c509575 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Wed, 3 Mar 2021 13:35:40 +0100
Subject: [PATCH 02/21] 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.
---
 src/basic/escape.c                            | 31 ++++------
 src/basic/escape.h                            | 17 +++---
 src/core/job.c                                |  2 +-
 .../environment-d-generator.c                 |  2 +-
 src/shared/bus-print-properties.c             |  2 +-
 src/shared/bus-wait-for-jobs.c                |  2 +-
 src/systemctl/systemctl-set-environment.c     |  2 +-
 src/test/test-escape.c                        | 61 ++++++++-----------
 8 files changed, 47 insertions(+), 72 deletions(-)

diff --git a/src/basic/escape.c b/src/basic/escape.c
index af785ecfa4..7a9a3b5616 100644
--- 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 691b6d802c..7ecae49e10 100644
--- 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 56c99f93eb..639dc352a6 100644
--- 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 1c51cf6b2c..5372f6cec2 100644
--- 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 2a712be733..dc16c2f763 100644
--- 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 51b71ecc2c..f325dd4e6d 100644
--- 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 bfd87c0cdd..a19e031dd3 100644
--- 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 3e410ca299..1e65368842 100644
--- 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[]) {

From 1129cd8a71680f72b9c65b1bf83b96f9581454a3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Wed, 3 Mar 2021 13:40:51 +0100
Subject: [PATCH 03/21] basic/escape: add mode where empty arguments are still
 shown as ""

For variables, FOO= is OK. But when quoting positional arguments, we want to
use something with quotes ("", '', or even $'') for an empty string.
---
 src/basic/escape.c     | 3 +++
 src/basic/escape.h     | 1 +
 src/test/test-escape.c | 4 ++++
 3 files changed, 8 insertions(+)

diff --git a/src/basic/escape.c b/src/basic/escape.c
index 7a9a3b5616..2de275f04e 100644
--- a/src/basic/escape.c
+++ b/src/basic/escape.c
@@ -504,6 +504,9 @@ char* shell_maybe_quote(const char *s, ShellEscapeFlags flags) {
          * string. Note that we treat benign UTF-8 characters as needing
          * escaping too, but that should be OK. */
 
+        if (FLAGS_SET(flags, SHELL_ESCAPE_EMPTY) && isempty(s))
+                return strdup("\"\""); /* We don't use $'' here in the POSIX mode. "" is fine too. */
+
         for (p = s; *p; p++)
                 if (*p <= ' ' ||
                     *p >= 127 ||
diff --git a/src/basic/escape.h b/src/basic/escape.h
index 7ecae49e10..3cb107ae90 100644
--- a/src/basic/escape.h
+++ b/src/basic/escape.h
@@ -38,6 +38,7 @@ typedef enum ShellEscapeFlags {
          * Tabs and newlines are escaped. */
 
         SHELL_ESCAPE_POSIX = 1 << 1, /* Use POSIX shell escape syntax (a string enclosed in $'') instead of plain quotes. */
+        SHELL_ESCAPE_EMPTY = 1 << 2, /* Format empty arguments as "". */
 } ShellEscapeFlags;
 
 char* cescape(const char *s);
diff --git a/src/test/test-escape.c b/src/test/test-escape.c
index 1e65368842..854a55174b 100644
--- a/src/test/test-escape.c
+++ b/src/test/test-escape.c
@@ -140,7 +140,9 @@ static void test_shell_maybe_quote_one(const char *s, ShellEscapeFlags flags, co
 static void test_shell_maybe_quote(void) {
 
         test_shell_maybe_quote_one("", 0, "");
+        test_shell_maybe_quote_one("", SHELL_ESCAPE_EMPTY, "\"\"");
         test_shell_maybe_quote_one("", SHELL_ESCAPE_POSIX, "");
+        test_shell_maybe_quote_one("", SHELL_ESCAPE_POSIX | SHELL_ESCAPE_EMPTY, "\"\"");
         test_shell_maybe_quote_one("\\", 0, "\"\\\\\"");
         test_shell_maybe_quote_one("\\", SHELL_ESCAPE_POSIX, "$'\\\\'");
         test_shell_maybe_quote_one("\"", 0, "\"\\\"\"");
@@ -156,7 +158,9 @@ static void test_shell_maybe_quote(void) {
         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_EMPTY, "\"foo\\$bar\"");
         test_shell_maybe_quote_one("foo$bar", SHELL_ESCAPE_POSIX, "$'foo$bar'");
+        test_shell_maybe_quote_one("foo$bar", SHELL_ESCAPE_POSIX | SHELL_ESCAPE_EMPTY, "$'foo$bar'");
 
         /* Note that current users disallow control characters, so this "test"
          * is here merely to establish current behaviour. If control characters

From 566d06ae508ae2f23c555506434028239b2b5b21 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Wed, 3 Mar 2021 13:47:55 +0100
Subject: [PATCH 04/21] basic/escape: always escape newlines in shell_escape()

shell_escape() is mostly used for mount paths and similar, where we assume
no newlines are present in the string. But if any were ever present, we
should escape them. So let's simplify the code by making this unconditional.
---
 src/basic/escape.c     | 9 ++++-----
 src/test/test-escape.c | 1 +
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/basic/escape.c b/src/basic/escape.c
index 2de275f04e..f45536d9bd 100644
--- a/src/basic/escape.c
+++ b/src/basic/escape.c
@@ -462,11 +462,11 @@ char* octescape(const char *s, size_t len) {
 
 }
 
-static char* strcpy_backslash_escaped(char *t, const char *s, const char *bad, bool escape_tab_nl) {
+static char* strcpy_backslash_escaped(char *t, const char *s, const char *bad) {
         assert(bad);
 
         for (; *s; s++) {
-                if (escape_tab_nl && IN_SET(*s, '\n', '\t')) {
+                if (IN_SET(*s, '\n', '\t')) {
                         *(t++) = '\\';
                         *(t++) = *s == '\n' ? 'n' : 't';
                         continue;
@@ -488,7 +488,7 @@ char* shell_escape(const char *s, const char *bad) {
         if (!r)
                 return NULL;
 
-        t = strcpy_backslash_escaped(r, s, bad, false);
+        t = strcpy_backslash_escaped(r, s, bad);
         *t = 0;
 
         return r;
@@ -530,8 +530,7 @@ char* shell_maybe_quote(const char *s, ShellEscapeFlags flags) {
         t = mempcpy(t, s, p - s);
 
         t = strcpy_backslash_escaped(t, p,
-                                     FLAGS_SET(flags, SHELL_ESCAPE_POSIX) ? SHELL_NEED_ESCAPE_POSIX : SHELL_NEED_ESCAPE,
-                                     true);
+                                     FLAGS_SET(flags, SHELL_ESCAPE_POSIX) ? SHELL_NEED_ESCAPE_POSIX : SHELL_NEED_ESCAPE);
 
         if (FLAGS_SET(flags, SHELL_ESCAPE_POSIX))
                 *(t++) = '\'';
diff --git a/src/test/test-escape.c b/src/test/test-escape.c
index 854a55174b..848aec37d8 100644
--- a/src/test/test-escape.c
+++ b/src/test/test-escape.c
@@ -127,6 +127,7 @@ static void test_shell_escape(void) {
         test_shell_escape_one("foobar", "", "foobar");
         test_shell_escape_one("foobar", "o", "f\\o\\obar");
         test_shell_escape_one("foo:bar,baz", ",:", "foo\\:bar\\,baz");
+        test_shell_escape_one("foo\nbar\nbaz", ",:", "foo\\nbar\\nbaz");
 }
 
 static void test_shell_maybe_quote_one(const char *s, ShellEscapeFlags flags, const char *expected) {

From 6302d38609b79042f90ee6f3e8a49bad6775e1a9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Wed, 3 Mar 2021 14:35:55 +0100
Subject: [PATCH 05/21] basic/string-util: split out helper function

---
 src/basic/string-util.c | 5 +----
 src/basic/string-util.h | 3 +++
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/basic/string-util.c b/src/basic/string-util.c
index 058eec54ff..c8ed87765f 100644
--- a/src/basic/string-util.c
+++ b/src/basic/string-util.c
@@ -277,10 +277,7 @@ bool string_has_cc(const char *p, const char *ok) {
                 if (ok && strchr(ok, *t))
                         continue;
 
-                if (*t > 0 && *t < ' ')
-                        return true;
-
-                if (*t == 127)
+                if (char_is_cc(*t))
                         return true;
         }
 
diff --git a/src/basic/string-util.h b/src/basic/string-util.h
index cb2881b64d..0dafe049ad 100644
--- a/src/basic/string-util.h
+++ b/src/basic/string-util.h
@@ -129,6 +129,9 @@ static inline bool _pure_ in_charset(const char *s, const char* charset) {
         return s[strspn(s, charset)] == '\0';
 }
 
+static inline bool char_is_cc(char p) {
+        return (p >= 0 && p < ' ') || p == 127;
+}
 bool string_has_cc(const char *p, const char *ok) _pure_;
 
 char *ellipsize_mem(const char *s, size_t old_length_bytes, size_t new_length_columns, unsigned percent);

From a01080ceb388912c140cd0b8391d70b883c0a17e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Wed, 3 Mar 2021 14:36:24 +0100
Subject: [PATCH 06/21] basic/string-util: inline iterator variable
 declarations

---
 src/basic/string-util.c | 32 ++++++++++----------------------
 1 file changed, 10 insertions(+), 22 deletions(-)

diff --git a/src/basic/string-util.c b/src/basic/string-util.c
index c8ed87765f..bcf39cd3ec 100644
--- a/src/basic/string-util.c
+++ b/src/basic/string-util.c
@@ -150,7 +150,7 @@ char *delete_chars(char *s, const char *bad) {
 }
 
 char *delete_trailing_chars(char *s, const char *bad) {
-        char *p, *c = s;
+        char *c = s;
 
         /* Drops all specified bad characters, at the end of the string */
 
@@ -160,7 +160,7 @@ char *delete_trailing_chars(char *s, const char *bad) {
         if (!bad)
                 bad = WHITESPACE;
 
-        for (p = s; *p; p++)
+        for (char *p = s; *p; p++)
                 if (!strchr(bad, *p))
                         c = p + 1;
 
@@ -193,34 +193,28 @@ char ascii_toupper(char x) {
 }
 
 char *ascii_strlower(char *t) {
-        char *p;
-
         assert(t);
 
-        for (p = t; *p; p++)
+        for (char *p = t; *p; p++)
                 *p = ascii_tolower(*p);
 
         return t;
 }
 
 char *ascii_strupper(char *t) {
-        char *p;
-
         assert(t);
 
-        for (p = t; *p; p++)
+        for (char *p = t; *p; p++)
                 *p = ascii_toupper(*p);
 
         return t;
 }
 
 char *ascii_strlower_n(char *t, size_t n) {
-        size_t i;
-
         if (n <= 0)
                 return t;
 
-        for (i = 0; i < n; i++)
+        for (size_t i = 0; i < n; i++)
                 t[i] = ascii_tolower(t[i]);
 
         return t;
@@ -252,10 +246,8 @@ int ascii_strcasecmp_nn(const char *a, size_t n, const char *b, size_t m) {
 }
 
 bool chars_intersect(const char *a, const char *b) {
-        const char *p;
-
         /* Returns true if any of the chars in a are in b. */
-        for (p = a; *p; p++)
+        for (const char *p = a; *p; p++)
                 if (strchr(b, *p))
                         return true;
 
@@ -263,8 +255,6 @@ bool chars_intersect(const char *a, const char *b) {
 }
 
 bool string_has_cc(const char *p, const char *ok) {
-        const char *t;
-
         assert(p);
 
         /*
@@ -273,7 +263,7 @@ bool string_has_cc(const char *p, const char *ok) {
          * considered OK.
          */
 
-        for (t = p; *t; t++) {
+        for (const char *t = p; *t; t++) {
                 if (ok && strchr(ok, *t))
                         continue;
 
@@ -465,7 +455,7 @@ char *cellescape(char *buf, size_t len, const char *s) {
          * very end.
          */
 
-        size_t i = 0, last_char_width[4] = {}, k = 0, j;
+        size_t i = 0, last_char_width[4] = {}, k = 0;
 
         assert(len > 0); /* at least a terminating NUL */
 
@@ -494,7 +484,7 @@ char *cellescape(char *buf, size_t len, const char *s) {
 
         /* Ellipsation is necessary. This means we might need to truncate the string again to make space for 4
          * characters ideally, but the buffer is shorter than that in the first place take what we can get */
-        for (j = 0; j < ELEMENTSOF(last_char_width); j++) {
+        for (size_t j = 0; j < ELEMENTSOF(last_char_width); j++) {
 
                 if (i + 4 <= len) /* nice, we reached our space goal */
                         break;
@@ -900,14 +890,12 @@ int free_and_strndup(char **p, const char *s, size_t l) {
 }
 
 bool string_is_safe(const char *p) {
-        const char *t;
-
         if (!p)
                 return false;
 
         /* Checks if the specified string contains no quotes or control characters */
 
-        for (t = p; *t; t++) {
+        for (const char *t = p; *t; t++) {
                 if (*t > 0 && *t < ' ') /* no control characters */
                         return false;
 

From 523e1b14a1d7af8e666a47bf980619d29128ca2b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Thu, 11 Mar 2021 11:40:57 +0100
Subject: [PATCH 07/21] basic/string-util: simplify how str_realloc() is used

All callers ignore failure anyway, so let's do that internally.
---
 src/basic/process-util.c |  2 +-
 src/basic/string-util.h  | 12 ++++--------
 src/basic/utf8.c         |  6 ++----
 3 files changed, 7 insertions(+), 13 deletions(-)

diff --git a/src/basic/process-util.c b/src/basic/process-util.c
index 7d4301eadb..902265ac21 100644
--- a/src/basic/process-util.c
+++ b/src/basic/process-util.c
@@ -179,7 +179,7 @@ int get_process_cmdline(pid_t pid, size_t max_columns, ProcessCmdlineFlags flags
         if (!ans)
                 return -ENOMEM;
 
-        (void) str_realloc(&ans);
+        ans = str_realloc(ans);
         *line = TAKE_PTR(ans);
         return 0;
 }
diff --git a/src/basic/string-util.h b/src/basic/string-util.h
index 0dafe049ad..1bc131f15b 100644
--- a/src/basic/string-util.h
+++ b/src/basic/string-util.h
@@ -217,17 +217,13 @@ static inline void *memory_startswith_no_case(const void *p, size_t sz, const ch
         return (uint8_t*) p + n;
 }
 
-static inline char* str_realloc(char **p) {
-        /* Reallocate *p to actual size */
+static inline char* str_realloc(char *p) {
+        /* Reallocate *p to actual size. Ignore failure, and return the original string on error. */
 
-        if (!*p)
+        if (!p)
                 return NULL;
 
-        char *t = realloc(*p, strlen(*p) + 1);
-        if (!t)
-                return NULL;
-
-        return (*p = t);
+        return realloc(p, strlen(p) + 1) ?: p;
 }
 
 char* string_erase(char *x);
diff --git a/src/basic/utf8.c b/src/basic/utf8.c
index 46c3a463b9..244b8ade93 100644
--- a/src/basic/utf8.c
+++ b/src/basic/utf8.c
@@ -196,8 +196,7 @@ char *utf8_escape_invalid(const char *str) {
         }
 
         *s = '\0';
-        (void) str_realloc(&p);
-        return p;
+        return str_realloc(p);
 }
 
 static int utf8_char_console_width(const char *str) {
@@ -282,8 +281,7 @@ char *utf8_escape_non_printable_full(const char *str, size_t console_width) {
 
  finish:
         *s = '\0';
-        (void) str_realloc(&p);
-        return p;
+        return str_realloc(p);
 }
 
 char *ascii_is_valid(const char *str) {

From 0089ab080036237b6e50afe99dea380fb2608f2c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Wed, 3 Mar 2021 14:56:23 +0100
Subject: [PATCH 08/21] basic/escape: escape control characters, but not utf-8,
 in shell quoting
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The comment in the code said that so far this didn't matter, but I want to use
shell quoting in more places where this will make a difference. So control
characters are now escaped. Normal utf-8 characters are passed through, it
is 2021 after all and pretty much everyone is (or should be) using utf-8.

While touching the code, change 'char *r' → 'char *buf', in line with modern
style.
---
 src/basic/escape.c     | 47 ++++++++++++++++++------------------------
 src/test/test-escape.c | 21 +++++++++++++------
 2 files changed, 35 insertions(+), 33 deletions(-)

diff --git a/src/basic/escape.c b/src/basic/escape.c
index f45536d9bd..45899b6b7c 100644
--- a/src/basic/escape.c
+++ b/src/basic/escape.c
@@ -465,62 +465,55 @@ char* octescape(const char *s, size_t len) {
 static char* strcpy_backslash_escaped(char *t, const char *s, const char *bad) {
         assert(bad);
 
-        for (; *s; s++) {
-                if (IN_SET(*s, '\n', '\t')) {
-                        *(t++) = '\\';
-                        *(t++) = *s == '\n' ? 'n' : 't';
-                        continue;
+        for (; *s; s++)
+                if (char_is_cc(*s))
+                        t += cescape_char(*s, t);
+                else {
+                        if (*s == '\\' || strchr(bad, *s))
+                                *(t++) = '\\';
+                        *(t++) = *s;
                 }
 
-                if (*s == '\\' || strchr(bad, *s))
-                        *(t++) = '\\';
-
-                *(t++) = *s;
-        }
-
         return t;
 }
 
 char* shell_escape(const char *s, const char *bad) {
-        char *r, *t;
+        char *buf, *t;
 
-        r = new(char, strlen(s)*2+1);
-        if (!r)
+        buf = new(char, strlen(s)*4+1);
+        if (!buf)
                 return NULL;
 
-        t = strcpy_backslash_escaped(r, s, bad);
+        t = strcpy_backslash_escaped(buf, s, bad);
         *t = 0;
 
-        return r;
+        return buf;
 }
 
 char* shell_maybe_quote(const char *s, ShellEscapeFlags flags) {
         const char *p;
-        char *r, *t;
+        char *buf, *t;
 
         assert(s);
 
-        /* Encloses a string in quotes if necessary to make it OK as a shell
-         * string. Note that we treat benign UTF-8 characters as needing
-         * escaping too, but that should be OK. */
+        /* Encloses a string in quotes if necessary to make it OK as a shell string. */
 
         if (FLAGS_SET(flags, SHELL_ESCAPE_EMPTY) && isempty(s))
                 return strdup("\"\""); /* We don't use $'' here in the POSIX mode. "" is fine too. */
 
         for (p = s; *p; p++)
-                if (*p <= ' ' ||
-                    *p >= 127 ||
-                    strchr(SHELL_NEED_QUOTES, *p))
+                if (char_is_cc(*p) ||
+                    strchr(WHITESPACE SHELL_NEED_QUOTES, *p))
                         break;
 
         if (!*p)
                 return strdup(s);
 
-        r = new(char, FLAGS_SET(flags, SHELL_ESCAPE_POSIX) + 1 + strlen(s)*2 + 1 + 1);
-        if (!r)
+        buf = new(char, FLAGS_SET(flags, SHELL_ESCAPE_POSIX) + 1 + strlen(s)*4 + 1 + 1);
+        if (!buf)
                 return NULL;
 
-        t = r;
+        t = buf;
         if (FLAGS_SET(flags, SHELL_ESCAPE_POSIX)) {
                 *(t++) = '$';
                 *(t++) = '\'';
@@ -538,5 +531,5 @@ char* shell_maybe_quote(const char *s, ShellEscapeFlags flags) {
                 *(t++) = '"';
         *t = 0;
 
-        return r;
+        return str_realloc(buf);
 }
diff --git a/src/test/test-escape.c b/src/test/test-escape.c
index 848aec37d8..eca66ea9db 100644
--- a/src/test/test-escape.c
+++ b/src/test/test-escape.c
@@ -118,6 +118,7 @@ static void test_shell_escape_one(const char *s, const char *bad, const char *ex
         _cleanup_free_ char *r;
 
         assert_se(r = shell_escape(s, bad));
+        log_debug("%s → %s (expected %s)", s, r, expected);
         assert_se(streq_ptr(r, expected));
 }
 
@@ -163,14 +164,22 @@ static void test_shell_maybe_quote(void) {
         test_shell_maybe_quote_one("foo$bar", SHELL_ESCAPE_POSIX, "$'foo$bar'");
         test_shell_maybe_quote_one("foo$bar", SHELL_ESCAPE_POSIX | SHELL_ESCAPE_EMPTY, "$'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", 0, "\"a\\nb\001\"");
-        test_shell_maybe_quote_one("a\nb\001", SHELL_ESCAPE_POSIX, "$'a\\nb\001'");
-
+        /* Exclamation mark is special in the interactive shell, but we don't treat it so. */
         test_shell_maybe_quote_one("foo!bar", 0, "\"foo!bar\"");
         test_shell_maybe_quote_one("foo!bar", SHELL_ESCAPE_POSIX, "$'foo!bar'");
+
+        /* Control characters and unicode */
+        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("głąb", 0, "głąb");
+        test_shell_maybe_quote_one("głąb", SHELL_ESCAPE_POSIX, "głąb");
+
+        test_shell_maybe_quote_one("głąb\002\003", 0, "\"głąb\\002\\003\"");
+        test_shell_maybe_quote_one("głąb\002\003", SHELL_ESCAPE_POSIX, "$'głąb\\002\\003'");
+
+        test_shell_maybe_quote_one("głąb\002\003rząd", 0, "\"głąb\\002\\003rząd\"");
+        test_shell_maybe_quote_one("głąb\002\003rząd", SHELL_ESCAPE_POSIX, "$'głąb\\002\\003rząd'");
 }
 
 int main(int argc, char *argv[]) {

From ad0e687c07ae5580d8bedde7c632e4a4d30d849f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Thu, 1 Apr 2021 15:23:02 +0200
Subject: [PATCH 09/21] basic/fileio: add a mode to read_full_virtual_file()
 where not the whole file is read

---
 src/basic/fileio.c     | 47 ++++++++++++++++++++++++++++--------------
 src/basic/fileio.h     |  7 ++++++-
 src/test/test-fileio.c | 10 +++++----
 3 files changed, 43 insertions(+), 21 deletions(-)

diff --git a/src/basic/fileio.c b/src/basic/fileio.c
index df30870a1a..90484a98c2 100644
--- a/src/basic/fileio.c
+++ b/src/basic/fileio.c
@@ -363,32 +363,38 @@ int verify_file(const char *fn, const char *blob, bool accept_extra_nl) {
         return 1;
 }
 
-int read_full_virtual_file(const char *filename, char **ret_contents, size_t *ret_size) {
+int read_virtual_file(const char *filename, size_t max_size, char **ret_contents, size_t *ret_size) {
         _cleanup_free_ char *buf = NULL;
         _cleanup_close_ int fd = -1;
-        struct stat st;
         size_t n, size;
         int n_retries;
 
         assert(ret_contents);
 
-        /* Virtual filesystems such as sysfs or procfs use kernfs, and kernfs can work
-         * with two sorts of virtual files. One sort uses "seq_file", and the results of
-         * the first read are buffered for the second read. The other sort uses "raw"
-         * reads which always go direct to the device. In the latter case, the content of
-         * the virtual file must be retrieved with a single read otherwise a second read
-         * might get the new value instead of finding EOF immediately. That's the reason
-         * why the usage of fread(3) is prohibited in this case as it always performs a
-         * second call to read(2) looking for EOF. See issue 13585. */
+        /* Virtual filesystems such as sysfs or procfs use kernfs, and kernfs can work with two sorts of
+         * virtual files. One sort uses "seq_file", and the results of the first read are buffered for the
+         * second read. The other sort uses "raw" reads which always go direct to the device. In the latter
+         * case, the content of the virtual file must be retrieved with a single read otherwise a second read
+         * might get the new value instead of finding EOF immediately. That's the reason why the usage of
+         * fread(3) is prohibited in this case as it always performs a second call to read(2) looking for
+         * EOF. See issue #13585.
+         *
+         * max_size specifies a limit on the bytes read. If max_size is SIZE_MAX, the full file is read. If
+         * the the full file is too large to read, an error is returned. For other values of max_size,
+         * *partial contents* may be returned. (Though the read is still done using one syscall.) */
 
         fd = open(filename, O_RDONLY|O_CLOEXEC);
         if (fd < 0)
                 return -errno;
 
+        assert(max_size <= READ_FULL_BYTES_MAX || max_size == SIZE_MAX);
+
         /* Limit the number of attempts to read the number of bytes returned by fstat(). */
         n_retries = 3;
 
         for (;;) {
+                struct stat st;
+
                 if (fstat(fd, &st) < 0)
                         return -errno;
 
@@ -398,13 +404,16 @@ int read_full_virtual_file(const char *filename, char **ret_contents, size_t *re
                 /* Be prepared for files from /proc which generally report a file size of 0. */
                 assert_cc(READ_FULL_BYTES_MAX < SSIZE_MAX);
                 if (st.st_size > 0) {
-                        if (st.st_size > READ_FULL_BYTES_MAX)
+                        if (st.st_size > SSIZE_MAX) /* Avoid overflow with 32-bit size_t and 64-bit off_t. */
+                                return -EFBIG;
+
+                        size = MIN((size_t) st.st_size, max_size);
+                        if (size > READ_FULL_BYTES_MAX)
                                 return -EFBIG;
 
-                        size = st.st_size;
                         n_retries--;
                 } else {
-                        size = READ_FULL_BYTES_MAX;
+                        size = MIN(READ_FULL_BYTES_MAX, max_size);
                         n_retries = 0;
                 }
 
@@ -412,7 +421,7 @@ int read_full_virtual_file(const char *filename, char **ret_contents, size_t *re
                 if (!buf)
                         return -ENOMEM;
                 /* Use a bigger allocation if we got it anyway, but not more than the limit. */
-                size = MIN(malloc_usable_size(buf) - 1, READ_FULL_BYTES_MAX);
+                size = MIN3(malloc_usable_size(buf) - 1, max_size, READ_FULL_BYTES_MAX);
 
                 for (;;) {
                         ssize_t k;
@@ -439,8 +448,14 @@ int read_full_virtual_file(const char *filename, char **ret_contents, size_t *re
                  * processing, let's try again either with a bigger guessed size or the new
                  * file size. */
 
-                if (n_retries <= 0)
-                        return st.st_size > 0 ? -EIO : -EFBIG;
+                if (n_retries <= 0) {
+                        if (max_size == SIZE_MAX)
+                                return st.st_size > 0 ? -EIO : -EFBIG;
+
+                        /* Accept a short read, but truncate it appropropriately. */
+                        n = MIN(n, max_size);
+                        break;
+                }
 
                 if (lseek(fd, 0, SEEK_SET) < 0)
                         return -errno;
diff --git a/src/basic/fileio.h b/src/basic/fileio.h
index 64a30ab7bb..2a6eef3790 100644
--- a/src/basic/fileio.h
+++ b/src/basic/fileio.h
@@ -64,7 +64,12 @@ int read_full_file_full(int dir_fd, const char *filename, uint64_t offset, size_
 static inline int read_full_file(const char *filename, char **ret_contents, size_t *ret_size) {
         return read_full_file_full(AT_FDCWD, filename, UINT64_MAX, SIZE_MAX, 0, NULL, ret_contents, ret_size);
 }
-int read_full_virtual_file(const char *filename, char **ret_contents, size_t *ret_size);
+
+int read_virtual_file(const char *filename, size_t max_size, char **ret_contents, size_t *ret_size);
+static inline int read_full_virtual_file(const char *filename, char **ret_contents, size_t *ret_size) {
+        return read_virtual_file(filename, SIZE_MAX, ret_contents, ret_size);
+}
+
 int read_full_stream_full(FILE *f, const char *filename, uint64_t offset, size_t size, ReadFullFileFlags flags, char **ret_contents, size_t *ret_size);
 static inline int read_full_stream(FILE *f, char **ret_contents, size_t *ret_size) {
         return read_full_stream_full(f, NULL, UINT64_MAX, SIZE_MAX, 0, ret_contents, ret_size);
diff --git a/src/test/test-fileio.c b/src/test/test-fileio.c
index c5c8116e96..f3859d9ffb 100644
--- a/src/test/test-fileio.c
+++ b/src/test/test-fileio.c
@@ -965,7 +965,7 @@ static void test_read_full_file_offset_size(void) {
         rbuf = mfree(rbuf);
 }
 
-static void test_read_full_virtual_file(void) {
+static void test_read_virtual_file(size_t max_size) {
         const char *filename;
         int r;
 
@@ -977,8 +977,8 @@ static void test_read_full_virtual_file(void) {
                 _cleanup_free_ char *buf = NULL;
                 size_t size = 0;
 
-                r = read_full_virtual_file(filename, &buf, &size);
-                log_info_errno(r, "read_full_virtual_file(\"%s\"): %m (%zu bytes)", filename, size);
+                r = read_virtual_file(filename, max_size, &buf, &size);
+                log_info_errno(r, "read_virtual_file(\"%s\", %zu): %m (%zu bytes)", filename, max_size, size);
                 assert_se(r == 0 || ERRNO_IS_PRIVILEGE(r) || r == -ENOENT);
         }
 }
@@ -1010,7 +1010,9 @@ int main(int argc, char *argv[]) {
         test_read_nul_string();
         test_read_full_file_socket();
         test_read_full_file_offset_size();
-        test_read_full_virtual_file();
+        test_read_virtual_file(20);
+        test_read_virtual_file(4096);
+        test_read_virtual_file(SIZE_MAX);
 
         return 0;
 }

From d12ccbc30252935b13bc2d4da51b55b096154afb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Thu, 1 Apr 2021 15:23:15 +0200
Subject: [PATCH 10/21] test-fileio: modernization

---
 src/test/test-fileio.c | 57 ++++++++++++++++++++++++++++++++----------
 1 file changed, 44 insertions(+), 13 deletions(-)

diff --git a/src/test/test-fileio.c b/src/test/test-fileio.c
index f3859d9ffb..bf763d7dd3 100644
--- a/src/test/test-fileio.c
+++ b/src/test/test-fileio.c
@@ -321,6 +321,8 @@ static void test_executable_is_script(void) {
         char *command;
         int r;
 
+        log_info("/* %s */", __func__);
+
         assert_se(fmkostemp_safe(t, "w", &f) == 0);
         fputs("#! /bin/script -a -b \ngoo goo", f);
         fflush(f);
@@ -346,6 +348,8 @@ static void test_status_field(void) {
         unsigned long long total = 0, buffers = 0;
         int r;
 
+        log_info("/* %s */", __func__);
+
         assert_se(get_proc_field("/proc/self/status", "Threads", WHITESPACE, &t) == 0);
         puts(t);
         assert_se(streq(t, "1"));
@@ -377,11 +381,11 @@ static void test_status_field(void) {
 }
 
 static void test_capeff(void) {
-        int pid, p;
+        log_info("/* %s */", __func__);
 
-        for (pid = 0; pid < 2; pid++) {
+        for (int pid = 0; pid < 2; pid++) {
                 _cleanup_free_ char *capeff = NULL;
-                int r;
+                int r, p;
 
                 r = get_process_capeff(0, &capeff);
                 log_info("capeff: '%s' (r=%d)", capeff, r);
@@ -402,6 +406,8 @@ static void test_write_string_stream(void) {
         int fd;
         char buf[64];
 
+        log_info("/* %s */", __func__);
+
         fd = mkostemp_safe(fn);
         assert_se(fd >= 0);
 
@@ -436,6 +442,8 @@ static void test_write_string_file(void) {
         char buf[64] = {};
         _cleanup_close_ int fd;
 
+        log_info("/* %s */", __func__);
+
         fd = mkostemp_safe(fn);
         assert_se(fd >= 0);
 
@@ -450,6 +458,8 @@ static void test_write_string_file_no_create(void) {
         _cleanup_close_ int fd;
         char buf[64] = {};
 
+        log_info("/* %s */", __func__);
+
         fd = mkostemp_safe(fn);
         assert_se(fd >= 0);
 
@@ -464,6 +474,8 @@ static void test_write_string_file_verify(void) {
         _cleanup_free_ char *buf = NULL, *buf2 = NULL;
         int r;
 
+        log_info("/* %s */", __func__);
+
         r = read_one_line_file("/proc/version", &buf);
         if (ERRNO_IS_PRIVILEGE(r))
                 return;
@@ -490,6 +502,8 @@ static void test_load_env_file_pairs(void) {
         _cleanup_strv_free_ char **l = NULL;
         char **k, **v;
 
+        log_info("/* %s */", __func__);
+
         fd = mkostemp_safe(fn);
         assert_se(fd >= 0);
 
@@ -531,6 +545,8 @@ static void test_search_and_fopen(void) {
         int fd, r;
         FILE *f;
 
+        log_info("/* %s */", __func__);
+
         fd = mkostemp_safe(name);
         assert_se(fd >= 0);
         close(fd);
@@ -562,6 +578,8 @@ static void test_search_and_fopen(void) {
 static void test_search_and_fopen_nulstr(void) {
         const char dirs[] = "/tmp/foo/bar\0/tmp\0";
 
+        log_info("/* %s */", __func__);
+
         _cleanup_(unlink_tempfilep) char name[] = "/tmp/test-search_and_fopen.XXXXXX";
         int fd, r;
         FILE *f;
@@ -595,12 +613,15 @@ static void test_writing_tmpfile(void) {
         _cleanup_free_ char *contents = NULL;
         size_t size;
         _cleanup_close_ int fd = -1;
-        struct iovec iov[3];
         int r;
 
-        iov[0] = IOVEC_MAKE_STRING("abc\n");
-        iov[1] = IOVEC_MAKE_STRING(ALPHANUMERICAL "\n");
-        iov[2] = IOVEC_MAKE_STRING("");
+        log_info("/* %s */", __func__);
+
+        struct iovec iov[] = {
+                IOVEC_MAKE_STRING("abc\n"),
+                IOVEC_MAKE_STRING(ALPHANUMERICAL "\n"),
+                IOVEC_MAKE_STRING(""),
+        };
 
         fd = mkostemp_safe(name);
         printf("tmpfile: %s", name);
@@ -617,6 +638,8 @@ static void test_writing_tmpfile(void) {
 static void test_tempfn(void) {
         char *ret = NULL, *p;
 
+        log_info("/* %s */", __func__);
+
         assert_se(tempfn_xxxxxx("/foo/bar/waldo", NULL, &ret) >= 0);
         assert_se(streq_ptr(ret, "/foo/bar/.#waldoXXXXXX"));
         free(ret);
@@ -659,8 +682,7 @@ static void test_fgetc(void) {
         _cleanup_fclose_ FILE *f = NULL;
         char c;
 
-        f = fmemopen_unlocked((void*) chars, sizeof(chars), "re");
-        assert_se(f);
+        assert_se(f = fmemopen_unlocked((void*) chars, sizeof(chars), "re"));
 
         for (size_t i = 0; i < sizeof(chars); i++) {
                 assert_se(safe_fgetc(f, &c) == 1);
@@ -753,9 +775,9 @@ static void test_read_line_one_file(FILE *f) {
 static void test_read_line(void) {
         _cleanup_fclose_ FILE *f = NULL;
 
-        f = fmemopen_unlocked((void*) buffer, sizeof(buffer), "re");
-        assert_se(f);
+        log_info("/* %s */", __func__);
 
+        assert_se(f = fmemopen_unlocked((void*) buffer, sizeof(buffer), "re"));
         test_read_line_one_file(f);
 }
 
@@ -764,6 +786,8 @@ static void test_read_line2(void) {
         int fd;
         _cleanup_fclose_ FILE *f = NULL;
 
+        log_info("/* %s */", __func__);
+
         fd = mkostemp_safe(name);
         assert_se(fd >= 0);
         assert_se((size_t) write(fd, buffer, sizeof(buffer)) == sizeof(buffer));
@@ -779,6 +803,8 @@ static void test_read_line3(void) {
         _cleanup_free_ char *line = NULL;
         int r;
 
+        log_info("/* %s */", __func__);
+
         f = fopen("/proc/uptime", "re");
         if (!f && IN_SET(errno, ENOENT, EPERM))
                 return;
@@ -811,10 +837,9 @@ static void test_read_line4(void) {
                 { 6, "foo\n\r\0" },
         };
 
-        size_t i;
         int r;
 
-        for (i = 0; i < ELEMENTSOF(eof_endings); i++) {
+        for (size_t i = 0; i < ELEMENTSOF(eof_endings); i++) {
                 _cleanup_fclose_ FILE *f = NULL;
                 _cleanup_free_ char *s = NULL;
 
@@ -839,6 +864,8 @@ static void test_read_nul_string(void) {
         _cleanup_fclose_ FILE *f = NULL;
         _cleanup_free_ char *s = NULL;
 
+        log_info("/* %s */", __func__);
+
         assert_se(f = fmemopen_unlocked((void*) test, sizeof(test)-1, "r"));
 
         assert_se(read_nul_string(f, LONG_LINE_MAX, &s) == 13 && streq_ptr(s, "string nr. 1"));
@@ -928,6 +955,8 @@ static void test_read_full_file_offset_size(void) {
         size_t rbuf_size;
         uint8_t buf[4711];
 
+        log_info("/* %s */", __func__);
+
         random_bytes(buf, sizeof(buf));
 
         assert_se(tempfn_random_child(NULL, NULL, &fn) >= 0);
@@ -969,6 +998,8 @@ static void test_read_virtual_file(size_t max_size) {
         const char *filename;
         int r;
 
+        log_info("/* %s (max_size=%zu) */", __func__, max_size);
+
         FOREACH_STRING(filename,
                        "/proc/1/cmdline",
                        "/etc/nsswitch.conf",

From b19f211698f2359464bd46c69a45390b6579b705 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Wed, 5 May 2021 12:41:25 +0200
Subject: [PATCH 11/21] basic/escape: flagsify xescape_full()

---
 src/basic/escape.c       | 15 ++++++++-------
 src/basic/escape.h       | 10 +++++++---
 src/basic/process-util.c |  2 +-
 src/test/test-escape.c   |  3 ++-
 4 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/src/basic/escape.c b/src/basic/escape.c
index 45899b6b7c..f579f15d87 100644
--- a/src/basic/escape.c
+++ b/src/basic/escape.c
@@ -360,13 +360,13 @@ int cunescape_length_with_prefix(const char *s, size_t length, const char *prefi
         return t - r;
 }
 
-char* xescape_full(const char *s, const char *bad, size_t console_width, bool eight_bits) {
+char* xescape_full(const char *s, const char *bad, size_t console_width, XEscapeFlags flags) {
         char *ans, *t, *prev, *prev2;
         const char *f;
 
         /* Escapes all chars in bad, in addition to \ and all special chars, in \xFF style escaping. May be
-         * reversed with cunescape(). If eight_bits is true, characters >= 127 are let through unchanged.
-         * This corresponds to non-ASCII printable characters in pre-unicode encodings.
+         * reversed with cunescape(). If XESCAPE_8_BIT is specified, characters >= 127 are let through
+         * unchanged. This corresponds to non-ASCII printable characters in pre-unicode encodings.
          *
          * If console_width is reached, output is truncated and "..." is appended. */
 
@@ -388,7 +388,8 @@ char* xescape_full(const char *s, const char *bad, size_t console_width, bool ei
                         return ans;
                 }
 
-                if ((unsigned char) *f < ' ' || (!eight_bits && (unsigned char) *f >= 127) ||
+                if ((unsigned char) *f < ' ' ||
+                    (!FLAGS_SET(flags, XESCAPE_8_BIT) && (unsigned char) *f >= 127) ||
                     *f == '\\' || strchr(bad, *f)) {
                         if ((size_t) (t - ans) + 4 > console_width)
                                 break;
@@ -427,9 +428,9 @@ char* xescape_full(const char *s, const char *bad, size_t console_width, bool ei
         return ans;
 }
 
-char* escape_non_printable_full(const char *str, size_t console_width, bool eight_bit) {
-        if (eight_bit)
-                return xescape_full(str, "", console_width, true);
+char* escape_non_printable_full(const char *str, size_t console_width, XEscapeFlags flags) {
+        if (FLAGS_SET(flags, XESCAPE_8_BIT))
+                return xescape_full(str, "", console_width, flags);
         else
                 return utf8_escape_non_printable_full(str, console_width);
 }
diff --git a/src/basic/escape.h b/src/basic/escape.h
index 3cb107ae90..945e7dc82c 100644
--- a/src/basic/escape.h
+++ b/src/basic/escape.h
@@ -54,12 +54,16 @@ static inline int cunescape(const char *s, UnescapeFlags flags, char **ret) {
 }
 int cunescape_one(const char *p, size_t length, char32_t *ret, bool *eight_bit, bool accept_nul);
 
-char* xescape_full(const char *s, const char *bad, size_t console_width, bool eight_bits);
+typedef enum XEscapeFlags {
+        XESCAPE_8_BIT          = 1 << 0,
+} XEscapeFlags;
+
+char* xescape_full(const char *s, const char *bad, size_t console_width, XEscapeFlags flags);
 static inline char* xescape(const char *s, const char *bad) {
-        return xescape_full(s, bad, SIZE_MAX, false);
+        return xescape_full(s, bad, SIZE_MAX, 0);
 }
 char* octescape(const char *s, size_t len);
-char* escape_non_printable_full(const char *str, size_t console_width, bool eight_bit);
+char* escape_non_printable_full(const char *str, size_t console_width, XEscapeFlags flags);
 
 char* shell_escape(const char *s, const char *bad);
 char* shell_maybe_quote(const char *s, ShellEscapeFlags flags);
diff --git a/src/basic/process-util.c b/src/basic/process-util.c
index 902265ac21..d7afc4fe5a 100644
--- a/src/basic/process-util.c
+++ b/src/basic/process-util.c
@@ -175,7 +175,7 @@ int get_process_cmdline(pid_t pid, size_t max_columns, ProcessCmdlineFlags flags
 
         bool eight_bit = (flags & PROCESS_CMDLINE_USE_LOCALE) && !is_locale_utf8();
 
-        ans = escape_non_printable_full(t, max_columns, eight_bit);
+        ans = escape_non_printable_full(t, max_columns, eight_bit * XESCAPE_8_BIT);
         if (!ans)
                 return -ENOMEM;
 
diff --git a/src/test/test-escape.c b/src/test/test-escape.c
index eca66ea9db..63f9306fb4 100644
--- a/src/test/test-escape.c
+++ b/src/test/test-escape.c
@@ -24,11 +24,12 @@ static void test_xescape_full(bool eight_bits) {
                 "a\\x62c\\x5c\"\\x08\\x0c\\x0a\\x0d\\x09\\x0b\\x07\\x03\\x7f\\x9c\\xcb" :
                 "a\\x62c\\x5c\"\\x08\\x0c\\x0a\\x0d\\x09\\x0b\\x07\\x03\177\234\313";
         const unsigned full_fit = !eight_bits ? 55 : 46;
+        XEscapeFlags flags = eight_bits * XESCAPE_8_BIT;
 
         for (unsigned i = 0; i < 60; i++) {
                 _cleanup_free_ char *t;
 
-                assert_se(t = xescape_full("abc\\\"\b\f\n\r\t\v\a\003\177\234\313", "b", i, eight_bits));
+                assert_se(t = xescape_full("abc\\\"\b\f\n\r\t\v\a\003\177\234\313", "b", i, flags));
 
                 log_info("%02d: %s", i, t);
 

From fc96e5c0536ae6d9d689a373b696f4fd3659f7d3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Wed, 5 May 2021 12:53:53 +0200
Subject: [PATCH 12/21] =?UTF-8?q?basic/escape:=20allow=20truncation=20mode?=
 =?UTF-8?q?=20where=20"=E2=80=A6"=20is=20always=20appended?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

So far we would append "…" or "..." when the string was wider than the specified
output width. But let's add a mode where the caller knows that the string being
passed is already truncated.

The condition for jumping back in utf8_escape_non_printable_full() was
off-by-one. But we only jumped to that label after doing a check with a
stronger condition, so I think it didn't matter. Now it matters because we'd
output the forced ellipsis one column too early.
---
 src/basic/escape.c     | 16 +++++++++++----
 src/basic/escape.h     |  1 +
 src/basic/utf8.c       | 12 +++++++----
 src/basic/utf8.h       |  4 ++--
 src/test/test-escape.c | 13 ++++++++++--
 src/test/test-utf8.c   | 45 ++++++++++++++++++++----------------------
 6 files changed, 55 insertions(+), 36 deletions(-)

diff --git a/src/basic/escape.c b/src/basic/escape.c
index f579f15d87..2a3a0e31a1 100644
--- a/src/basic/escape.c
+++ b/src/basic/escape.c
@@ -368,7 +368,8 @@ char* xescape_full(const char *s, const char *bad, size_t console_width, XEscape
          * reversed with cunescape(). If XESCAPE_8_BIT is specified, characters >= 127 are let through
          * unchanged. This corresponds to non-ASCII printable characters in pre-unicode encodings.
          *
-         * If console_width is reached, output is truncated and "..." is appended. */
+         * If console_width is reached, or XESCAPE_FORCE_ELLIPSIS is set, output is truncated and "..." is
+         * appended. */
 
         if (console_width == 0)
                 return strdup("");
@@ -380,10 +381,15 @@ char* xescape_full(const char *s, const char *bad, size_t console_width, XEscape
         memset(ans, '_', MIN(strlen(s), console_width) * 4);
         ans[MIN(strlen(s), console_width) * 4] = 0;
 
+        bool force_ellipsis = FLAGS_SET(flags, XESCAPE_FORCE_ELLIPSIS);
+
         for (f = s, t = prev = prev2 = ans; ; f++) {
                 char *tmp_t = t;
 
                 if (!*f) {
+                        if (force_ellipsis)
+                                break;
+
                         *t = 0;
                         return ans;
                 }
@@ -391,7 +397,7 @@ char* xescape_full(const char *s, const char *bad, size_t console_width, XEscape
                 if ((unsigned char) *f < ' ' ||
                     (!FLAGS_SET(flags, XESCAPE_8_BIT) && (unsigned char) *f >= 127) ||
                     *f == '\\' || strchr(bad, *f)) {
-                        if ((size_t) (t - ans) + 4 > console_width)
+                        if ((size_t) (t - ans) + 4 + 3 * force_ellipsis > console_width)
                                 break;
 
                         *(t++) = '\\';
@@ -399,7 +405,7 @@ char* xescape_full(const char *s, const char *bad, size_t console_width, XEscape
                         *(t++) = hexchar(*f >> 4);
                         *(t++) = hexchar(*f);
                 } else {
-                        if ((size_t) (t - ans) + 1 > console_width)
+                        if ((size_t) (t - ans) + 1 + 3 * force_ellipsis > console_width)
                                 break;
 
                         *(t++) = *f;
@@ -432,7 +438,9 @@ char* escape_non_printable_full(const char *str, size_t console_width, XEscapeFl
         if (FLAGS_SET(flags, XESCAPE_8_BIT))
                 return xescape_full(str, "", console_width, flags);
         else
-                return utf8_escape_non_printable_full(str, console_width);
+                return utf8_escape_non_printable_full(str,
+                                                      console_width,
+                                                      FLAGS_SET(flags, XESCAPE_FORCE_ELLIPSIS));
 }
 
 char* octescape(const char *s, size_t len) {
diff --git a/src/basic/escape.h b/src/basic/escape.h
index 945e7dc82c..907b572bd4 100644
--- a/src/basic/escape.h
+++ b/src/basic/escape.h
@@ -56,6 +56,7 @@ int cunescape_one(const char *p, size_t length, char32_t *ret, bool *eight_bit,
 
 typedef enum XEscapeFlags {
         XESCAPE_8_BIT          = 1 << 0,
+        XESCAPE_FORCE_ELLIPSIS = 1 << 1,
 } XEscapeFlags;
 
 char* xescape_full(const char *s, const char *bad, size_t console_width, XEscapeFlags flags);
diff --git a/src/basic/utf8.c b/src/basic/utf8.c
index 244b8ade93..63fc9f71d1 100644
--- a/src/basic/utf8.c
+++ b/src/basic/utf8.c
@@ -212,7 +212,7 @@ static int utf8_char_console_width(const char *str) {
         return unichar_iswide(c) ? 2 : 1;
 }
 
-char *utf8_escape_non_printable_full(const char *str, size_t console_width) {
+char *utf8_escape_non_printable_full(const char *str, size_t console_width, bool force_ellipsis) {
         char *p, *s, *prev_s;
         size_t n = 0; /* estimated print width */
 
@@ -229,8 +229,12 @@ char *utf8_escape_non_printable_full(const char *str, size_t console_width) {
                 int len;
                 char *saved_s = s;
 
-                if (!*str) /* done! */
-                        goto finish;
+                if (!*str) { /* done! */
+                        if (force_ellipsis)
+                                goto truncation;
+                        else
+                                goto finish;
+                }
 
                 len = utf8_encoded_valid_unichar(str, SIZE_MAX);
                 if (len > 0) {
@@ -274,7 +278,7 @@ char *utf8_escape_non_printable_full(const char *str, size_t console_width) {
 
  truncation:
         /* Try to go back one if we don't have enough space for the ellipsis */
-        if (n + 1 >= console_width)
+        if (n + 1 > console_width)
                 s = prev_s;
 
         s = mempcpy(s, "…", strlen("…"));
diff --git a/src/basic/utf8.h b/src/basic/utf8.h
index 219ca89184..b0e969f655 100644
--- a/src/basic/utf8.h
+++ b/src/basic/utf8.h
@@ -25,9 +25,9 @@ bool utf8_is_printable_newline(const char* str, size_t length, bool allow_newlin
 #define utf8_is_printable(str, length) utf8_is_printable_newline(str, length, true)
 
 char *utf8_escape_invalid(const char *s);
-char *utf8_escape_non_printable_full(const char *str, size_t console_width);
+char *utf8_escape_non_printable_full(const char *str, size_t console_width, bool force_ellipsis);
 static inline char *utf8_escape_non_printable(const char *str) {
-        return utf8_escape_non_printable_full(str, SIZE_MAX);
+        return utf8_escape_non_printable_full(str, SIZE_MAX, false);
 }
 
 size_t utf8_encode_unichar(char *out_utf8, char32_t g);
diff --git a/src/test/test-escape.c b/src/test/test-escape.c
index 63f9306fb4..991b135a33 100644
--- a/src/test/test-escape.c
+++ b/src/test/test-escape.c
@@ -27,11 +27,11 @@ static void test_xescape_full(bool eight_bits) {
         XEscapeFlags flags = eight_bits * XESCAPE_8_BIT;
 
         for (unsigned i = 0; i < 60; i++) {
-                _cleanup_free_ char *t;
+                _cleanup_free_ char *t, *q;
 
                 assert_se(t = xescape_full("abc\\\"\b\f\n\r\t\v\a\003\177\234\313", "b", i, flags));
 
-                log_info("%02d: %s", i, t);
+                log_info("%02d: <%s>", i, t);
 
                 if (i >= full_fit)
                         assert_se(streq(t, escaped));
@@ -45,6 +45,15 @@ static void test_xescape_full(bool eight_bits) {
                         assert_se(strlen(t) == i);
                         assert_se(strneq(t, "...", i));
                 }
+
+                assert_se(q = xescape_full("abc\\\"\b\f\n\r\t\v\a\003\177\234\313", "b", i,
+                                           flags | XESCAPE_FORCE_ELLIPSIS));
+
+                log_info("%02d: <%s>", i, q);
+                if (i > 0)
+                        assert_se(endswith(q, "."));
+                assert(strlen(q) <= i);
+                assert(strlen(q) + 3 >= strlen(t));
         }
 }
 
diff --git a/src/test/test-utf8.c b/src/test/test-utf8.c
index 042b94634b..cdbdfcb054 100644
--- a/src/test/test-utf8.c
+++ b/src/test/test-utf8.c
@@ -136,32 +136,29 @@ static void test_utf8_escape_non_printable(void) {
 static void test_utf8_escape_non_printable_full(void) {
         log_info("/* %s */", __func__);
 
-        for (size_t i = 0; i < 20; i++) {
-                _cleanup_free_ char *p;
+        const char *s;
+        FOREACH_STRING(s,
+                       "goo goo goo",       /* ASCII */
+                       "\001 \019\20\a",    /* control characters */
+                       "\xef\xbf\x30\x13")  /* misplaced continuation bytes followed by a digit and cc */
+                for (size_t cw = 0; cw < 22; cw++) {
+                        _cleanup_free_ char *p, *q;
+                        size_t ew;
 
-                p = utf8_escape_non_printable_full("goo goo goo", i);
-                puts(p);
-                assert_se(utf8_is_valid(p));
-                assert_se(utf8_console_width(p) <= i);
-        }
+                        p = utf8_escape_non_printable_full(s, cw, false);
+                        ew = utf8_console_width(p);
+                        log_debug("%02zu \"%s\" (%zu wasted)", cw, p, cw - ew);
+                        assert_se(utf8_is_valid(p));
+                        assert_se(ew <= cw);
 
-        for (size_t i = 0; i < 20; i++) {
-                _cleanup_free_ char *p;
-
-                p = utf8_escape_non_printable_full("\001 \019\20\a", i);
-                puts(p);
-                assert_se(utf8_is_valid(p));
-                assert_se(utf8_console_width(p) <= i);
-        }
-
-        for (size_t i = 0; i < 20; i++) {
-                _cleanup_free_ char *p;
-
-                p = utf8_escape_non_printable_full("\xef\xbf\x30\x13", i);
-                puts(p);
-                assert_se(utf8_is_valid(p));
-                assert_se(utf8_console_width(p) <= i);
-        }
+                        q = utf8_escape_non_printable_full(s, cw, true);
+                        ew = utf8_console_width(q);
+                        log_debug("   \"%s\" (%zu wasted)", q, cw - ew);
+                        assert_se(utf8_is_valid(q));
+                        assert_se(ew <= cw);
+                        if (cw > 0)
+                                assert_se(endswith(q, "…"));
+                }
 }
 
 static void test_utf16_to_utf8(void) {

From 82208a9949ff96abfd41ea3dd969fa7501ee4686 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Fri, 2 Apr 2021 11:09:09 +0200
Subject: [PATCH 13/21] test-utf8: hide most output by default

Unless one is working on the code, there is little reason to write most
of the output. So let's hide it unless requested with SYSTEMD_LOG_LEVEL=debug.
---
 src/test/test-utf8.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/src/test/test-utf8.c b/src/test/test-utf8.c
index cdbdfcb054..4ba9ca8439 100644
--- a/src/test/test-utf8.c
+++ b/src/test/test-utf8.c
@@ -3,6 +3,7 @@
 #include "alloc-util.h"
 #include "string-util.h"
 #include "strv.h"
+#include "tests.h"
 #include "utf8.h"
 #include "util.h"
 
@@ -91,15 +92,15 @@ static void test_utf8_escape_invalid(void) {
         log_info("/* %s */", __func__);
 
         p1 = utf8_escape_invalid("goo goo goo");
-        puts(p1);
+        log_debug("\"%s\"", p1);
         assert_se(utf8_is_valid(p1));
 
         p2 = utf8_escape_invalid("\341\204\341\204");
-        puts(p2);
+        log_debug("\"%s\"", p2);
         assert_se(utf8_is_valid(p2));
 
         p3 = utf8_escape_invalid("\341\204");
-        puts(p3);
+        log_debug("\"%s\"", p3);
         assert_se(utf8_is_valid(p3));
 }
 
@@ -109,27 +110,27 @@ static void test_utf8_escape_non_printable(void) {
         log_info("/* %s */", __func__);
 
         p1 = utf8_escape_non_printable("goo goo goo");
-        puts(p1);
+        log_debug("\"%s\"", p1);
         assert_se(utf8_is_valid(p1));
 
         p2 = utf8_escape_non_printable("\341\204\341\204");
-        puts(p2);
+        log_debug("\"%s\"", p2);
         assert_se(utf8_is_valid(p2));
 
         p3 = utf8_escape_non_printable("\341\204");
-        puts(p3);
+        log_debug("\"%s\"", p3);
         assert_se(utf8_is_valid(p3));
 
         p4 = utf8_escape_non_printable("ąę\n가너도루\n1234\n\341\204\341\204\n\001 \019\20\a");
-        puts(p4);
+        log_debug("\"%s\"", p4);
         assert_se(utf8_is_valid(p4));
 
         p5 = utf8_escape_non_printable("\001 \019\20\a");
-        puts(p5);
+        log_debug("\"%s\"", p5);
         assert_se(utf8_is_valid(p5));
 
         p6 = utf8_escape_non_printable("\xef\xbf\x30\x13");
-        puts(p6);
+        log_debug("\"%s\"", p6);
         assert_se(utf8_is_valid(p6));
 }
 
@@ -232,6 +233,9 @@ static void test_utf8_to_utf16(void) {
 }
 
 int main(int argc, char *argv[]) {
+        log_show_color(true);
+        test_setup_logging(LOG_INFO);
+
         test_utf8_n_is_valid();
         test_utf8_is_valid();
         test_utf8_is_printable();

From 61977664e9d186287d0b85a26256cf82ae89fcbf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Tue, 30 Mar 2021 19:42:36 +0200
Subject: [PATCH 14/21] basic/process-util: allow quoting of commandlines

Since the new functionality is controlled by an option, this causes no change
in output yet, except tests.

The login in the old branch of !(flags & PROCESS_CMDLINE_QUOTE) is essentially
unmodified. But there is an important difference in behaviour: instead of
unconditionally reading the whole virtual file, we now read only 'max_columns'
bytes. This makes out code to write process lists quite a bit more efficient
when there are processes with long command lines.
---
 src/basic/fileio.c           |   7 +-
 src/basic/process-util.c     | 131 ++++++++++++++++++++++++++---------
 src/basic/process-util.h     |   1 +
 src/test/test-process-util.c |  45 +++++++++++-
 4 files changed, 148 insertions(+), 36 deletions(-)

diff --git a/src/basic/fileio.c b/src/basic/fileio.c
index 90484a98c2..024eb29bb9 100644
--- a/src/basic/fileio.c
+++ b/src/basic/fileio.c
@@ -368,6 +368,7 @@ int read_virtual_file(const char *filename, size_t max_size, char **ret_contents
         _cleanup_close_ int fd = -1;
         size_t n, size;
         int n_retries;
+        bool truncated = false;
 
         assert(ret_contents);
 
@@ -381,7 +382,8 @@ int read_virtual_file(const char *filename, size_t max_size, char **ret_contents
          *
          * max_size specifies a limit on the bytes read. If max_size is SIZE_MAX, the full file is read. If
          * the the full file is too large to read, an error is returned. For other values of max_size,
-         * *partial contents* may be returned. (Though the read is still done using one syscall.) */
+         * *partial contents* may be returned. (Though the read is still done using one syscall.)
+         * Returns 0 on partial success, 1 if untruncated contents were read. */
 
         fd = open(filename, O_RDONLY|O_CLOEXEC);
         if (fd < 0)
@@ -454,6 +456,7 @@ int read_virtual_file(const char *filename, size_t max_size, char **ret_contents
 
                         /* Accept a short read, but truncate it appropropriately. */
                         n = MIN(n, max_size);
+                        truncated = true;
                         break;
                 }
 
@@ -484,7 +487,7 @@ int read_virtual_file(const char *filename, size_t max_size, char **ret_contents
         buf[n] = 0;
         *ret_contents = TAKE_PTR(buf);
 
-        return 0;
+        return !truncated;
 }
 
 int read_full_stream_full(
diff --git a/src/basic/process-util.c b/src/basic/process-util.c
index d7afc4fe5a..fd708eed98 100644
--- a/src/basic/process-util.c
+++ b/src/basic/process-util.c
@@ -123,64 +123,133 @@ int get_process_comm(pid_t pid, char **ret) {
         return 0;
 }
 
-int get_process_cmdline(pid_t pid, size_t max_columns, ProcessCmdlineFlags flags, char **line) {
-        _cleanup_free_ char *t = NULL, *ans = NULL;
+static int get_process_cmdline_nulstr(
+                pid_t pid,
+                size_t max_size,
+                ProcessCmdlineFlags flags,
+                char **ret,
+                size_t *ret_size) {
+
         const char *p;
+        char *t;
         size_t k;
         int r;
 
-        assert(line);
-        assert(pid >= 0);
-
-        /* Retrieves a process' command line. Replaces non-utf8 bytes by replacement character (�). If
-         * max_columns is != -1 will return a string of the specified console width at most, abbreviated with
-         * an ellipsis. If PROCESS_CMDLINE_COMM_FALLBACK is specified in flags and the process has no command
-         * line set (the case for kernel threads), or has a command line that resolves to the empty string
-         * will return the "comm" name of the process instead. This will use at most _SC_ARG_MAX bytes of
-         * input data.
+        /* Retrieves a process' command line as a "sized nulstr", i.e. possibly without the last NUL, but
+         * with a specified size.
          *
-         * Returns -ESRCH if the process doesn't exist, and -ENOENT if the process has no command line (and
-         * comm_fallback is false). Returns 0 and sets *line otherwise. */
+         * If PROCESS_CMDLINE_COMM_FALLBACK is specified in flags and the process has no command line set
+         * (the case for kernel threads), or has a command line that resolves to the empty string, will
+         * return the "comm" name of the process instead. This will use at most _SC_ARG_MAX bytes of input
+         * data.
+         *
+         * Returns an error, 0 if output was read but is truncated, 1 otherwise.
+         */
 
         p = procfs_file_alloca(pid, "cmdline");
-        r = read_full_virtual_file(p, &t, &k);
+        r = read_virtual_file(p, max_size, &t, &k); /* Let's assume that each input byte results in >= 1
+                                                     * columns of output. We ignore zero-width codepoints. */
         if (r == -ENOENT)
                 return -ESRCH;
         if (r < 0)
                 return r;
 
-        if (k > 0) {
-                /* Arguments are separated by NULs. Let's replace those with spaces. */
-                for (size_t i = 0; i < k - 1; i++)
-                        if (t[i] == '\0')
-                                t[i] = ' ';
-        } else {
+        if (k == 0) {
+                t = mfree(t);
+
                 if (!(flags & PROCESS_CMDLINE_COMM_FALLBACK))
                         return -ENOENT;
 
                 /* Kernel threads have no argv[] */
-                _cleanup_free_ char *t2 = NULL;
+                _cleanup_free_ char *comm = NULL;
 
-                r = get_process_comm(pid, &t2);
+                r = get_process_comm(pid, &comm);
                 if (r < 0)
                         return r;
 
-                free(t);
-                t = strjoin("[", t2, "]");
+                t = strjoin("[", comm, "]");
                 if (!t)
                         return -ENOMEM;
+
+                k = strlen(t);
+                r = k <= max_size;
+                if (r == 0) /* truncation */
+                        t[max_size] = '\0';
         }
 
-        delete_trailing_chars(t, WHITESPACE);
+        *ret = t;
+        *ret_size = k;
+        return r;
+}
 
-        bool eight_bit = (flags & PROCESS_CMDLINE_USE_LOCALE) && !is_locale_utf8();
+int get_process_cmdline(pid_t pid, size_t max_columns, ProcessCmdlineFlags flags, char **line) {
+        _cleanup_free_ char *t = NULL;
+        size_t k;
+        char *ans;
 
-        ans = escape_non_printable_full(t, max_columns, eight_bit * XESCAPE_8_BIT);
-        if (!ans)
-                return -ENOMEM;
+        assert(line);
+        assert(pid >= 0);
 
-        ans = str_realloc(ans);
-        *line = TAKE_PTR(ans);
+        /* Retrieve adn format a commandline. See above for discussion of retrieval options.
+         *
+         * There are two main formatting modes:
+         *
+         * - when PROCESS_CMDLINE_QUOTE is specified, output is quoted in C/Python style. If no shell special
+         *   characters are present, this output can be copy-pasted into the terminal to execute. UTF-8
+         *   output is assumed.
+         *
+         * - otherwise, a compact non-roundtrippable form is returned. Non-UTF8 bytes are replaced by �. The
+         *   returned string is of the specified console width at most, abbreviated with an ellipsis.
+         *
+         * Returns -ESRCH if the process doesn't exist, and -ENOENT if the process has no command line (and
+         * PROCESS_CMDLINE_COMM_FALLBACK is not specified). Returns 0 and sets *line otherwise. */
+
+        int full = get_process_cmdline_nulstr(pid, max_columns, flags, &t, &k);
+        if (full < 0)
+                return full;
+
+        if (flags & PROCESS_CMDLINE_QUOTE) {
+                assert(!(flags & PROCESS_CMDLINE_USE_LOCALE));
+
+                _cleanup_strv_free_ char **args = NULL;
+
+                args = strv_parse_nulstr(t, k);
+                if (!args)
+                        return -ENOMEM;
+
+                for (size_t i = 0; args[i]; i++) {
+                        char *e;
+
+                        e = shell_maybe_quote(args[i], SHELL_ESCAPE_EMPTY);
+                        if (!e)
+                                return -ENOMEM;
+
+                        free_and_replace(args[i], e);
+                }
+
+                ans = strv_join(args, " ");
+                if (!ans)
+                        return -ENOMEM;
+
+        } else {
+                /* Arguments are separated by NULs. Let's replace those with spaces. */
+                for (size_t i = 0; i < k - 1; i++)
+                        if (t[i] == '\0')
+                                t[i] = ' ';
+
+                delete_trailing_chars(t, WHITESPACE);
+
+                bool eight_bit = (flags & PROCESS_CMDLINE_USE_LOCALE) && !is_locale_utf8();
+
+                ans = escape_non_printable_full(t, max_columns,
+                                                eight_bit * XESCAPE_8_BIT | !full * XESCAPE_FORCE_ELLIPSIS);
+                if (!ans)
+                        return -ENOMEM;
+
+                ans = str_realloc(ans);
+        }
+
+        *line = ans;
         return 0;
 }
 
diff --git a/src/basic/process-util.h b/src/basic/process-util.h
index ddce7bd272..3121d82d3f 100644
--- a/src/basic/process-util.h
+++ b/src/basic/process-util.h
@@ -35,6 +35,7 @@
 typedef enum ProcessCmdlineFlags {
         PROCESS_CMDLINE_COMM_FALLBACK = 1 << 0,
         PROCESS_CMDLINE_USE_LOCALE    = 1 << 1,
+        PROCESS_CMDLINE_QUOTE         = 1 << 2,
 } ProcessCmdlineFlags;
 
 int get_process_comm(pid_t pid, char **name);
diff --git a/src/test/test-process-util.c b/src/test/test-process-util.c
index 7f0a771ba7..5f148c1522 100644
--- a/src/test/test-process-util.c
+++ b/src/test/test-process-util.c
@@ -248,9 +248,15 @@ static void test_get_process_cmdline_harder(void) {
         assert_se(get_process_cmdline(0, SIZE_MAX, 0, &line) == -ENOENT);
 
         assert_se(get_process_cmdline(0, SIZE_MAX, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        log_info("'%s'", line);
         assert_se(streq(line, "[testa]"));
         line = mfree(line);
 
+        assert_se(get_process_cmdline(0, SIZE_MAX, PROCESS_CMDLINE_COMM_FALLBACK | PROCESS_CMDLINE_QUOTE, &line) >= 0);
+        log_info("'%s'", line);
+        assert_se(streq(line, "\"[testa]\"")); /* quoting is enabled here */
+        line = mfree(line);
+
         assert_se(get_process_cmdline(0, 0, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
         log_info("'%s'", line);
         assert_se(streq(line, ""));
@@ -288,6 +294,8 @@ static void test_get_process_cmdline_harder(void) {
         assert_se(streq(line, "[testa]"));
         line = mfree(line);
 
+        /* Test with multiple arguments that don't require quoting */
+
         assert_se(write(fd, "foo\0bar", 8) == 8);
 
         assert_se(get_process_cmdline(0, SIZE_MAX, 0, &line) >= 0);
@@ -390,6 +398,32 @@ static void test_get_process_cmdline_harder(void) {
         assert_se(streq(line, "[aaaa bbbb …"));
         line = mfree(line);
 
+        /* Test with multiple arguments that do require quoting */
+
+#define CMDLINE1 "foo\0'bar'\0\"bar$\"\0x y z\0!``\0"
+#define EXPECT1  "foo \"'bar'\" \"\\\"bar\\$\\\"\" \"x y z\" \"!\\`\\`\" \"\""
+        assert_se(lseek(fd, SEEK_SET, 0) == 0);
+        assert_se(write(fd, CMDLINE1, sizeof CMDLINE1) == sizeof CMDLINE1);
+        assert_se(ftruncate(fd, sizeof CMDLINE1) == 0);
+
+        assert_se(get_process_cmdline(0, SIZE_MAX, PROCESS_CMDLINE_QUOTE, &line) >= 0);
+        log_info("got: ==%s==", line);
+        log_info("exp: ==%s==", EXPECT1);
+        assert_se(streq(line, EXPECT1));
+        line = mfree(line);
+
+#define CMDLINE2 "foo\0\1\2\3\0\0"
+#define EXPECT2  "foo \"\\001\\002\\003\" \"\" \"\""
+        assert_se(lseek(fd, SEEK_SET, 0) == 0);
+        assert_se(write(fd, CMDLINE2, sizeof CMDLINE2) == sizeof CMDLINE2);
+        assert_se(ftruncate(fd, sizeof CMDLINE2) == 0);
+
+        assert_se(get_process_cmdline(0, SIZE_MAX, PROCESS_CMDLINE_QUOTE, &line) >= 0);
+        log_info("got: ==%s==", line);
+        log_info("exp: ==%s==", EXPECT2);
+        assert_se(streq(line, EXPECT2));
+        line = mfree(line);
+
         safe_close(fd);
         _exit(EXIT_SUCCESS);
 }
@@ -403,6 +437,8 @@ static void test_rename_process_now(const char *p, int ret) {
                   (ret == 0 && r >= 0) ||
                   (ret > 0 && r > 0));
 
+        log_info_errno(r, "rename_process(%s): %m", p);
+
         if (r < 0)
                 return;
 
@@ -425,9 +461,12 @@ static void test_rename_process_now(const char *p, int ret) {
                 if (r == 0 && detect_container() > 0)
                         log_info("cmdline = <%s> (not verified, Running in unprivileged container?)", cmdline);
                 else {
-                        log_info("cmdline = <%s>", cmdline);
-                        assert_se(strneq(p, cmdline, STRLEN("test-process-util")));
-                        assert_se(startswith(p, cmdline));
+                        log_info("cmdline = <%s> (expected <%.*s>)", cmdline, (int) strlen("test-process-util"), p);
+
+                        bool skip = cmdline[0] == '"'; /* A shortcut to check if the string is quoted */
+
+                        assert_se(strneq(cmdline + skip, p, strlen("test-process-util")));
+                        assert_se(startswith(cmdline + skip, p));
                 }
         } else
                 log_info("cmdline = <%s> (not verified)", cmdline);

From daceaabe1f87de85a404499d95c6f351b83fb3d9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Fri, 2 Apr 2021 11:35:23 +0200
Subject: [PATCH 15/21] test-process-util: add more debug logging but hide most
 of it by default

It makes little sense to always print the stuff that is fully deterministic
and verified by asserts. It can be opted-in with $SYSTEMD_LOG_LEVEL when
developing the tests or debugging a failure.
---
 src/test/test-process-util.c | 75 ++++++++++++++++++++++++++++--------
 1 file changed, 60 insertions(+), 15 deletions(-)

diff --git a/src/test/test-process-util.c b/src/test/test-process-util.c
index 5f148c1522..383e9ae3c5 100644
--- a/src/test/test-process-util.c
+++ b/src/test/test-process-util.c
@@ -43,6 +43,8 @@ static void test_get_process_comm(pid_t pid) {
         dev_t h;
         int r;
 
+        log_info("/* %s */", __func__);
+
         xsprintf(path, "/proc/"PID_FMT"/comm", pid);
 
         if (stat(path, &st) == 0) {
@@ -91,12 +93,12 @@ static void test_get_process_comm(pid_t pid) {
 static void test_get_process_comm_escape_one(const char *input, const char *output) {
         _cleanup_free_ char *n = NULL;
 
-        log_info("input: <%s> — output: <%s>", input, output);
+        log_debug("input: <%s> — output: <%s>", input, output);
 
         assert_se(prctl(PR_SET_NAME, input) >= 0);
         assert_se(get_process_comm(0, &n) >= 0);
 
-        log_info("got: <%s>", n);
+        log_debug("got: <%s>", n);
 
         assert_se(streq_ptr(n, output));
 }
@@ -104,6 +106,8 @@ static void test_get_process_comm_escape_one(const char *input, const char *outp
 static void test_get_process_comm_escape(void) {
         _cleanup_free_ char *saved = NULL;
 
+        log_info("/* %s */", __func__);
+
         assert_se(get_process_comm(0, &saved) >= 0);
 
         test_get_process_comm_escape_one("", "");
@@ -140,6 +144,8 @@ static void test_pid_is_unwaited(void) {
 static void test_pid_is_alive(void) {
         pid_t pid;
 
+        log_info("/* %s */", __func__);
+
         pid = fork();
         assert_se(pid >= 0);
         if (pid == 0) {
@@ -155,6 +161,7 @@ static void test_pid_is_alive(void) {
 }
 
 static void test_personality(void) {
+        log_info("/* %s */", __func__);
 
         assert_se(personality_to_string(PER_LINUX));
         assert_se(!personality_to_string(PERSONALITY_INVALID));
@@ -183,6 +190,8 @@ static void test_get_process_cmdline_harder(void) {
         _cleanup_free_ char *line = NULL;
         pid_t pid;
 
+        log_info("/* %s */", __func__);
+
         if (geteuid() != 0) {
                 log_info("Skipping %s: not root", __func__);
                 return;
@@ -248,17 +257,17 @@ static void test_get_process_cmdline_harder(void) {
         assert_se(get_process_cmdline(0, SIZE_MAX, 0, &line) == -ENOENT);
 
         assert_se(get_process_cmdline(0, SIZE_MAX, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
-        log_info("'%s'", line);
+        log_debug("'%s'", line);
         assert_se(streq(line, "[testa]"));
         line = mfree(line);
 
         assert_se(get_process_cmdline(0, SIZE_MAX, PROCESS_CMDLINE_COMM_FALLBACK | PROCESS_CMDLINE_QUOTE, &line) >= 0);
-        log_info("'%s'", line);
+        log_debug("'%s'", line);
         assert_se(streq(line, "\"[testa]\"")); /* quoting is enabled here */
         line = mfree(line);
 
         assert_se(get_process_cmdline(0, 0, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
-        log_info("'%s'", line);
+        log_debug("'%s'", line);
         assert_se(streq(line, ""));
         line = mfree(line);
 
@@ -299,7 +308,7 @@ static void test_get_process_cmdline_harder(void) {
         assert_se(write(fd, "foo\0bar", 8) == 8);
 
         assert_se(get_process_cmdline(0, SIZE_MAX, 0, &line) >= 0);
-        log_info("'%s'", line);
+        log_debug("'%s'", line);
         assert_se(streq(line, "foo bar"));
         line = mfree(line);
 
@@ -309,71 +318,87 @@ static void test_get_process_cmdline_harder(void) {
 
         assert_se(write(fd, "quux", 4) == 4);
         assert_se(get_process_cmdline(0, SIZE_MAX, 0, &line) >= 0);
-        log_info("'%s'", line);
+        log_debug("'%s'", line);
         assert_se(streq(line, "foo bar quux"));
         line = mfree(line);
 
         assert_se(get_process_cmdline(0, SIZE_MAX, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        log_debug("'%s'", line);
         assert_se(streq(line, "foo bar quux"));
         line = mfree(line);
 
         assert_se(get_process_cmdline(0, 1, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        log_debug("'%s'", line);
         assert_se(streq(line, "…"));
         line = mfree(line);
 
         assert_se(get_process_cmdline(0, 2, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        log_debug("'%s'", line);
         assert_se(streq(line, "f…"));
         line = mfree(line);
 
         assert_se(get_process_cmdline(0, 3, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        log_debug("'%s'", line);
         assert_se(streq(line, "fo…"));
         line = mfree(line);
 
         assert_se(get_process_cmdline(0, 4, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        log_debug("'%s'", line);
         assert_se(streq(line, "foo…"));
         line = mfree(line);
 
         assert_se(get_process_cmdline(0, 5, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        log_debug("'%s'", line);
         assert_se(streq(line, "foo …"));
         line = mfree(line);
 
         assert_se(get_process_cmdline(0, 6, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        log_debug("'%s'", line);
         assert_se(streq(line, "foo b…"));
         line = mfree(line);
 
         assert_se(get_process_cmdline(0, 7, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        log_debug("'%s'", line);
         assert_se(streq(line, "foo ba…"));
         line = mfree(line);
 
         assert_se(get_process_cmdline(0, 8, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        log_debug("'%s'", line);
         assert_se(streq(line, "foo bar…"));
         line = mfree(line);
 
         assert_se(get_process_cmdline(0, 9, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        log_debug("'%s'", line);
         assert_se(streq(line, "foo bar …"));
         line = mfree(line);
 
         assert_se(get_process_cmdline(0, 10, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        log_debug("'%s'", line);
         assert_se(streq(line, "foo bar q…"));
         line = mfree(line);
 
         assert_se(get_process_cmdline(0, 11, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        log_debug("'%s'", line);
         assert_se(streq(line, "foo bar qu…"));
         line = mfree(line);
 
         assert_se(get_process_cmdline(0, 12, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        log_debug("'%s'", line);
         assert_se(streq(line, "foo bar quux"));
         line = mfree(line);
 
         assert_se(get_process_cmdline(0, 13, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        log_debug("'%s'", line);
         assert_se(streq(line, "foo bar quux"));
         line = mfree(line);
 
         assert_se(get_process_cmdline(0, 14, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        log_debug("'%s'", line);
         assert_se(streq(line, "foo bar quux"));
         line = mfree(line);
 
         assert_se(get_process_cmdline(0, 1000, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        log_debug("'%s'", line);
         assert_se(streq(line, "foo bar quux"));
         line = mfree(line);
 
@@ -383,18 +408,22 @@ static void test_get_process_cmdline_harder(void) {
         assert_se(get_process_cmdline(0, SIZE_MAX, 0, &line) == -ENOENT);
 
         assert_se(get_process_cmdline(0, SIZE_MAX, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        log_debug("'%s'", line);
         assert_se(streq(line, "[aaaa bbbb cccc]"));
         line = mfree(line);
 
         assert_se(get_process_cmdline(0, 10, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        log_debug("'%s'", line);
         assert_se(streq(line, "[aaaa bbb…"));
         line = mfree(line);
 
         assert_se(get_process_cmdline(0, 11, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        log_debug("'%s'", line);
         assert_se(streq(line, "[aaaa bbbb…"));
         line = mfree(line);
 
         assert_se(get_process_cmdline(0, 12, PROCESS_CMDLINE_COMM_FALLBACK, &line) >= 0);
+        log_debug("'%s'", line);
         assert_se(streq(line, "[aaaa bbbb …"));
         line = mfree(line);
 
@@ -407,8 +436,8 @@ static void test_get_process_cmdline_harder(void) {
         assert_se(ftruncate(fd, sizeof CMDLINE1) == 0);
 
         assert_se(get_process_cmdline(0, SIZE_MAX, PROCESS_CMDLINE_QUOTE, &line) >= 0);
-        log_info("got: ==%s==", line);
-        log_info("exp: ==%s==", EXPECT1);
+        log_debug("got: ==%s==", line);
+        log_debug("exp: ==%s==", EXPECT1);
         assert_se(streq(line, EXPECT1));
         line = mfree(line);
 
@@ -419,8 +448,8 @@ static void test_get_process_cmdline_harder(void) {
         assert_se(ftruncate(fd, sizeof CMDLINE2) == 0);
 
         assert_se(get_process_cmdline(0, SIZE_MAX, PROCESS_CMDLINE_QUOTE, &line) >= 0);
-        log_info("got: ==%s==", line);
-        log_info("exp: ==%s==", EXPECT2);
+        log_debug("got: ==%s==", line);
+        log_debug("exp: ==%s==", EXPECT2);
         assert_se(streq(line, EXPECT2));
         line = mfree(line);
 
@@ -432,12 +461,14 @@ static void test_rename_process_now(const char *p, int ret) {
         _cleanup_free_ char *comm = NULL, *cmdline = NULL;
         int r;
 
+        log_info("/* %s */", __func__);
+
         r = rename_process(p);
         assert_se(r == ret ||
                   (ret == 0 && r >= 0) ||
                   (ret > 0 && r > 0));
 
-        log_info_errno(r, "rename_process(%s): %m", p);
+        log_debug_errno(r, "rename_process(%s): %m", p);
 
         if (r < 0)
                 return;
@@ -449,7 +480,7 @@ static void test_rename_process_now(const char *p, int ret) {
 #endif
 
         assert_se(get_process_comm(0, &comm) >= 0);
-        log_info("comm = <%s>", comm);
+        log_debug("comm = <%s>", comm);
         assert_se(strneq(comm, p, TASK_COMM_LEN-1));
         /* We expect comm to be at most 16 bytes (TASK_COMM_LEN). The kernel may raise this limit in the
          * future. We'd only check the initial part, at least until we recompile, but this will still pass. */
@@ -476,6 +507,8 @@ static void test_rename_process_one(const char *p, int ret) {
         siginfo_t si;
         pid_t pid;
 
+        log_info("/* %s */", __func__);
+
         pid = fork();
         assert_se(pid >= 0);
 
@@ -530,6 +563,8 @@ static void test_getpid_cached(void) {
         siginfo_t si;
         pid_t a, b, c, d, e, f, child;
 
+        log_info("/* %s */", __func__);
+
         a = raw_getpid();
         b = getpid_cached();
         c = getpid();
@@ -566,6 +601,8 @@ static void test_getpid_measure(void) {
         unsigned long long i;
         usec_t t, q;
 
+        log_info("/* %s */", __func__);
+
         t = now(CLOCK_MONOTONIC);
         for (i = 0; i < MEASURE_ITERATIONS; i++)
                 (void) getpid();
@@ -586,6 +623,8 @@ static void test_safe_fork(void) {
         pid_t pid;
         int r;
 
+        log_info("/* %s */", __func__);
+
         BLOCK_SIGNALS(SIGCHLD);
 
         r = safe_fork("(test-child)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_DEATHSIG|FORK_NULL_STDIO|FORK_REOPEN_LOG, &pid);
@@ -634,6 +673,8 @@ static void test_ioprio_class_from_to_string_one(const char *val, int expected)
 }
 
 static void test_ioprio_class_from_to_string(void) {
+        log_info("/* %s */", __func__);
+
         test_ioprio_class_from_to_string_one("none", IOPRIO_CLASS_NONE);
         test_ioprio_class_from_to_string_one("realtime", IOPRIO_CLASS_RT);
         test_ioprio_class_from_to_string_one("best-effort", IOPRIO_CLASS_BE);
@@ -649,7 +690,10 @@ static void test_ioprio_class_from_to_string(void) {
 static void test_setpriority_closest(void) {
         int r;
 
-        r = safe_fork("(test-setprio)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_DEATHSIG|FORK_WAIT|FORK_LOG, NULL);
+        log_info("/* %s */", __func__);
+
+        r = safe_fork("(test-setprio)",
+                      FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_DEATHSIG|FORK_WAIT|FORK_LOG, NULL);
         assert_se(r >= 0);
 
         if (r == 0) {
@@ -733,7 +777,8 @@ static void test_setpriority_closest(void) {
 }
 
 int main(int argc, char *argv[]) {
-        test_setup_logging(LOG_DEBUG);
+        log_show_color(true);
+        test_setup_logging(LOG_INFO);
 
         save_argc_argv(argc, argv);
 

From 07468a16e46dba9da8b9c8593e0c2ee5bfec3c69 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Fri, 2 Apr 2021 11:44:48 +0200
Subject: [PATCH 16/21] test-process-util: run fewer getpid() tests

Significant time was spent in the getpid() measurement code, which is not very
important.  So let's optimize this a bit by running the slower version less
times, and only running both tests a lesser amount of times unless slow tests
are enabled.

This gives the better accuracy then before in slow mode, and still reasonable
accuracy in fast mode without a noticable slowdown.
---
 src/test/test-process-util.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/src/test/test-process-util.c b/src/test/test-process-util.c
index 383e9ae3c5..957d23ffc5 100644
--- a/src/test/test-process-util.c
+++ b/src/test/test-process-util.c
@@ -595,27 +595,28 @@ static void test_getpid_cached(void) {
         assert_se(si.si_code == CLD_EXITED);
 }
 
-#define MEASURE_ITERATIONS (10000000LLU)
-
 static void test_getpid_measure(void) {
-        unsigned long long i;
         usec_t t, q;
 
-        log_info("/* %s */", __func__);
+        unsigned long long iterations = slow_tests_enabled() ? 1000000 : 1000;
+
+        log_info("/* %s (%llu iterations) */", __func__, iterations);
 
         t = now(CLOCK_MONOTONIC);
-        for (i = 0; i < MEASURE_ITERATIONS; i++)
+        for (unsigned long long i = 0; i < iterations; i++)
                 (void) getpid();
         q = now(CLOCK_MONOTONIC) - t;
 
-        log_info(" glibc getpid(): %lf µs each\n", (double) q / MEASURE_ITERATIONS);
+        log_info(" glibc getpid(): %lf µs each\n", (double) q / iterations);
+
+        iterations *= 50; /* _cached() is about 50 times faster, so we need more iterations */
 
         t = now(CLOCK_MONOTONIC);
-        for (i = 0; i < MEASURE_ITERATIONS; i++)
+        for (unsigned long long i = 0; i < iterations; i++)
                 (void) getpid_cached();
         q = now(CLOCK_MONOTONIC) - t;
 
-        log_info("getpid_cached(): %lf µs each\n", (double) q / MEASURE_ITERATIONS);
+        log_info("getpid_cached(): %lf µs each\n", (double) q / iterations);
 }
 
 static void test_safe_fork(void) {

From 99009ed0f490b89fa14ca323a9ffb9b963980822 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Thu, 11 Mar 2021 00:10:02 +0100
Subject: [PATCH 17/21] basic/process-util: add mode where posix shell escape
 is used for quoting

The new flag is not used, except in tests, so no functional change yet.

This way, the command as shown can be copied-and-pasted into the shell
in more cases. For simple cases, shell quoting with "" is enough. But
$'' is needed when there are control characters in the command.
---
 src/basic/process-util.c     |  7 +++++--
 src/basic/process-util.h     |  1 +
 src/test/test-process-util.c | 14 ++++++++++++++
 3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/src/basic/process-util.c b/src/basic/process-util.c
index fd708eed98..1b8e663efe 100644
--- a/src/basic/process-util.c
+++ b/src/basic/process-util.c
@@ -208,7 +208,10 @@ int get_process_cmdline(pid_t pid, size_t max_columns, ProcessCmdlineFlags flags
         if (full < 0)
                 return full;
 
-        if (flags & PROCESS_CMDLINE_QUOTE) {
+        if (flags & (PROCESS_CMDLINE_QUOTE | PROCESS_CMDLINE_QUOTE_POSIX)) {
+                ShellEscapeFlags shflags = SHELL_ESCAPE_EMPTY |
+                        FLAGS_SET(flags, PROCESS_CMDLINE_QUOTE_POSIX) * SHELL_ESCAPE_POSIX;
+
                 assert(!(flags & PROCESS_CMDLINE_USE_LOCALE));
 
                 _cleanup_strv_free_ char **args = NULL;
@@ -220,7 +223,7 @@ int get_process_cmdline(pid_t pid, size_t max_columns, ProcessCmdlineFlags flags
                 for (size_t i = 0; args[i]; i++) {
                         char *e;
 
-                        e = shell_maybe_quote(args[i], SHELL_ESCAPE_EMPTY);
+                        e = shell_maybe_quote(args[i], shflags);
                         if (!e)
                                 return -ENOMEM;
 
diff --git a/src/basic/process-util.h b/src/basic/process-util.h
index 3121d82d3f..8ce6d60f39 100644
--- a/src/basic/process-util.h
+++ b/src/basic/process-util.h
@@ -36,6 +36,7 @@ typedef enum ProcessCmdlineFlags {
         PROCESS_CMDLINE_COMM_FALLBACK = 1 << 0,
         PROCESS_CMDLINE_USE_LOCALE    = 1 << 1,
         PROCESS_CMDLINE_QUOTE         = 1 << 2,
+        PROCESS_CMDLINE_QUOTE_POSIX   = 1 << 3,
 } ProcessCmdlineFlags;
 
 int get_process_comm(pid_t pid, char **name);
diff --git a/src/test/test-process-util.c b/src/test/test-process-util.c
index 957d23ffc5..f79df46d29 100644
--- a/src/test/test-process-util.c
+++ b/src/test/test-process-util.c
@@ -431,6 +431,7 @@ static void test_get_process_cmdline_harder(void) {
 
 #define CMDLINE1 "foo\0'bar'\0\"bar$\"\0x y z\0!``\0"
 #define EXPECT1  "foo \"'bar'\" \"\\\"bar\\$\\\"\" \"x y z\" \"!\\`\\`\" \"\""
+#define EXPECT1p  "foo $'\\'bar\\'' $'\"bar$\"' $'x y z' $'!``' \"\""
         assert_se(lseek(fd, SEEK_SET, 0) == 0);
         assert_se(write(fd, CMDLINE1, sizeof CMDLINE1) == sizeof CMDLINE1);
         assert_se(ftruncate(fd, sizeof CMDLINE1) == 0);
@@ -441,8 +442,15 @@ static void test_get_process_cmdline_harder(void) {
         assert_se(streq(line, EXPECT1));
         line = mfree(line);
 
+        assert_se(get_process_cmdline(0, SIZE_MAX, PROCESS_CMDLINE_QUOTE_POSIX, &line) >= 0);
+        log_debug("got: ==%s==", line);
+        log_debug("exp: ==%s==", EXPECT1p);
+        assert_se(streq(line, EXPECT1p));
+        line = mfree(line);
+
 #define CMDLINE2 "foo\0\1\2\3\0\0"
 #define EXPECT2  "foo \"\\001\\002\\003\" \"\" \"\""
+#define EXPECT2p  "foo $'\\001\\002\\003' \"\" \"\""
         assert_se(lseek(fd, SEEK_SET, 0) == 0);
         assert_se(write(fd, CMDLINE2, sizeof CMDLINE2) == sizeof CMDLINE2);
         assert_se(ftruncate(fd, sizeof CMDLINE2) == 0);
@@ -453,6 +461,12 @@ static void test_get_process_cmdline_harder(void) {
         assert_se(streq(line, EXPECT2));
         line = mfree(line);
 
+        assert_se(get_process_cmdline(0, SIZE_MAX, PROCESS_CMDLINE_QUOTE_POSIX, &line) >= 0);
+        log_debug("got: ==%s==", line);
+        log_debug("exp: ==%s==", EXPECT2p);
+        assert_se(streq(line, EXPECT2p));
+        line = mfree(line);
+
         safe_close(fd);
         _exit(EXIT_SUCCESS);
 }

From 510c7a953ea5e12d8f2e4ab243dec546eb0325a1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Thu, 1 Apr 2021 16:46:01 +0200
Subject: [PATCH 18/21] test-process-util: add test that prints all cmdlines

---
 src/test/test-process-util.c | 49 ++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/src/test/test-process-util.c b/src/test/test-process-util.c
index f79df46d29..ca21eadaba 100644
--- a/src/test/test-process-util.c
+++ b/src/test/test-process-util.c
@@ -15,6 +15,8 @@
 #include "alloc-util.h"
 #include "architecture.h"
 #include "errno-util.h"
+#include "errno-list.h"
+#include "dirent-util.h"
 #include "fd-util.h"
 #include "log.h"
 #include "macro.h"
@@ -90,6 +92,52 @@ static void test_get_process_comm(pid_t pid) {
         log_info("PID"PID_FMT" $PATH: '%s'", pid, strna(i));
 }
 
+static void test_get_process_cmdline_one(pid_t pid) {
+        _cleanup_free_ char *c = NULL, *d = NULL, *e = NULL, *f = NULL, *g = NULL, *h = NULL;
+        int r;
+
+        r = get_process_cmdline(pid, SIZE_MAX, 0, &c);
+        log_info("PID "PID_FMT": %s", pid, r >= 0 ? c : errno_to_name(r));
+
+        r = get_process_cmdline(pid, SIZE_MAX, PROCESS_CMDLINE_COMM_FALLBACK, &d);
+        log_info("      %s", r >= 0 ? d : errno_to_name(r));
+
+        r = get_process_cmdline(pid, SIZE_MAX, PROCESS_CMDLINE_QUOTE, &e);
+        log_info("      %s", r >= 0 ? e : errno_to_name(r));
+
+        r = get_process_cmdline(pid, SIZE_MAX, PROCESS_CMDLINE_QUOTE | PROCESS_CMDLINE_COMM_FALLBACK, &f);
+        log_info("      %s", r >= 0 ? f : errno_to_name(r));
+
+        r = get_process_cmdline(pid, SIZE_MAX, PROCESS_CMDLINE_QUOTE_POSIX, &g);
+        log_info("      %s", r >= 0 ? g : errno_to_name(r));
+
+        r = get_process_cmdline(pid, SIZE_MAX, PROCESS_CMDLINE_QUOTE_POSIX | PROCESS_CMDLINE_COMM_FALLBACK, &h);
+        log_info("      %s", r >= 0 ? h : errno_to_name(r));
+}
+
+static void test_get_process_cmdline(void) {
+        _cleanup_closedir_ DIR *d = NULL;
+        struct dirent *de;
+
+        log_info("/* %s */", __func__);
+
+        assert_se(d = opendir("/proc"));
+
+        FOREACH_DIRENT(de, d, return) {
+                pid_t pid;
+
+                dirent_ensure_type(d, de);
+
+                if (de->d_type != DT_DIR)
+                        continue;
+
+                if (parse_pid(de->d_name, &pid) < 0)
+                        continue;
+
+                test_get_process_cmdline_one(pid);
+        }
+}
+
 static void test_get_process_comm_escape_one(const char *input, const char *output) {
         _cleanup_free_ char *n = NULL;
 
@@ -808,6 +856,7 @@ int main(int argc, char *argv[]) {
         }
 
         test_get_process_comm_escape();
+        test_get_process_cmdline();
         test_pid_is_unwaited();
         test_pid_is_alive();
         test_personality();

From 4e3fbc133eb099acada182e3049d278023d91360 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Wed, 3 Mar 2021 15:30:04 +0100
Subject: [PATCH 19/21] man: add an example of coredumpctl output

People like examples. Also shows off the new quoted command line.
---
 man/coredumpctl.xml | 36 +++++++++++++++++++++++++++++++++---
 1 file changed, 33 insertions(+), 3 deletions(-)

diff --git a/man/coredumpctl.xml b/man/coredumpctl.xml
index 2b602b71fa..68ba6dc9f2 100644
--- a/man/coredumpctl.xml
+++ b/man/coredumpctl.xml
@@ -354,10 +354,40 @@ Fri … 552351 1000 1000 SIGSEGV present  /usr/lib64/firefox/firefox 28.7M
     </example>
 
     <example>
-      <title>Show information about a process that dumped core,
-      matching by its PID 6654</title>
+      <title>Show information about a core dump matched by PID</title>
 
-      <programlisting>$ coredumpctl info 6654</programlisting>
+      <programlisting>$ coredumpctl info 6654
+           PID: 6654 (bash)
+           UID: 1000 (user)
+           GID: 1000 (user)
+        Signal: 11 (SEGV)
+     Timestamp: Mon 2021-01-01 00:00:01 CET (20s ago)
+  Command Line: bash -c kill -SEGV $$
+    Executable: /usr/bin/bash
+ Control Group: /user.slice/user-1000.slice/…
+          Unit: user@1000.service
+     User Unit: vte-spawn-….scope
+         Slice: user-1000.slice
+     Owner UID: 1000 (user)
+       Boot ID: …
+    Machine ID: …
+      Hostname: …
+       Storage: /var/lib/systemd/coredump/core.bash.1000.….zst (present)
+     Disk Size: 51.7K
+       Message: Process 130414 (bash) of user 1000 dumped core.
+
+                Stack trace of thread 130414:
+                #0  0x00007f398142358b kill (libc.so.6 + 0x3d58b)
+                #1  0x0000558c2c7fda09 kill_builtin (bash + 0xb1a09)
+                #2  0x0000558c2c79dc59 execute_builtin.lto_priv.0 (bash + 0x51c59)
+                #3  0x0000558c2c79709c execute_simple_command (bash + 0x4b09c)
+                #4  0x0000558c2c798408 execute_command_internal (bash + 0x4c408)
+                #5  0x0000558c2c7f6bdc parse_and_execute (bash + 0xaabdc)
+                #6  0x0000558c2c85415c run_one_command.isra.0 (bash + 0x10815c)
+                #7  0x0000558c2c77d040 main (bash + 0x31040)
+                #8  0x00007f398140db75 __libc_start_main (libc.so.6 + 0x27b75)
+                #9  0x0000558c2c77dd1e _start (bash + 0x31d1e)
+</programlisting>
     </example>
 
     <example>

From 5dd55303f4d7e4294e0d2f1098232bd0fa305832 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Fri, 2 Apr 2021 13:52:56 +0200
Subject: [PATCH 20/21] coredump: use "POSIX quotes" for cmdline

$ coredumpctl info |grep Command
  Command Line: bash -c kill -SEGV $$      (before)
  Command Line: bash -c "kill -SEGV \$\$"  (road not taken, C quotes)
  Command Line: bash -c $'kill -SEGV $$'   (now, POSIX quotes)

Before we wouldn't use any quoting, making it impossible to figure how the
command line was split into arguments. We could use "normal" quotes, but this
has the disadvantage that the commandline *looks* like it could be pasted into
the terminal and executed, but this is not true: various non-printable
characters cannot be expressed in this quoting style. (This is not visible in
this example). Thus, "POSIX quotes" are used, which should allow any command
line to be expressed acurrately and pasted directly into a shell prompt to
reexecute.

I wonder if we should another field in the coredump entry that simply shows the
original cmdline with embedded NULs, in the original /proc/*/cmdline
format. This would allow clients to format the data as they see fit. But I
think we'd want to keep the serialized form anyway, for backwards compatibility.
---
 man/coredumpctl.xml     | 2 +-
 src/coredump/coredump.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/man/coredumpctl.xml b/man/coredumpctl.xml
index 68ba6dc9f2..6ceed41b05 100644
--- a/man/coredumpctl.xml
+++ b/man/coredumpctl.xml
@@ -362,7 +362,7 @@ Fri … 552351 1000 1000 SIGSEGV present  /usr/lib64/firefox/firefox 28.7M
            GID: 1000 (user)
         Signal: 11 (SEGV)
      Timestamp: Mon 2021-01-01 00:00:01 CET (20s ago)
-  Command Line: bash -c kill -SEGV $$
+  Command Line: bash -c $'kill -SEGV $$'
     Executable: /usr/bin/bash
  Control Group: /user.slice/user-1000.slice/…
           Unit: user@1000.service
diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c
index 2fb2404500..a16be3c448 100644
--- a/src/coredump/coredump.c
+++ b/src/coredump/coredump.c
@@ -665,7 +665,7 @@ static int get_process_container_parent_cmdline(pid_t pid, char** cmdline) {
         if (r < 0)
                 return r;
 
-        r = get_process_cmdline(container_pid, SIZE_MAX, 0, cmdline);
+        r = get_process_cmdline(container_pid, SIZE_MAX, PROCESS_CMDLINE_QUOTE_POSIX, cmdline);
         if (r < 0)
                 return r;
 
@@ -1115,7 +1115,7 @@ static int gather_pid_metadata(struct iovec_wrapper *iovw, Context *context) {
         if (sd_pid_get_slice(pid, &t) >= 0)
                 (void) iovw_put_string_field_free(iovw, "COREDUMP_SLICE=", t);
 
-        if (get_process_cmdline(pid, SIZE_MAX, 0, &t) >= 0)
+        if (get_process_cmdline(pid, SIZE_MAX, PROCESS_CMDLINE_QUOTE_POSIX, &t) >= 0)
                 (void) iovw_put_string_field_free(iovw, "COREDUMP_CMDLINE=", t);
 
         if (cg_pid_get_path_shifted(pid, NULL, &t) >= 0)

From 2f960b3858eeefb3d27621291b94b72809e288e4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Fri, 2 Apr 2021 14:11:10 +0200
Subject: [PATCH 21/21] core,journald: use quoted commandlines

I think quoting is more useful than not quoting. Without, arguments with
whitespace cannot be split correctly.

Unlike in coredump, "normal" quoting is used in those two cases. This output is
mostly for informational purposes, so the more readable quoting seems apropriate.

dbus GetProcesses:
$ busctl --user call org.freedesktop.systemd1 /org/freedesktop/systemd1/unit/run_2dr4450e1ae73944194bb6593fcfd255fbe_2eservice org.freedesktop.systemd1.Service GetProcesses
a(sus) 2
"/user.slice/user-1000.slice/user@1000.service/app.slice/run-r4450e1ae73944194bb6593fcfd255fbe.service" 131494 "/usr/bin/bash -c \"sleep 100; sleep 20\""
"/user.slice/user-1000.slice/user@1000.service/app.slice/run-r4450e1ae73944194bb6593fcfd255fbe.service" 131496 "sleep 100"
---
 src/core/dbus-unit.c           | 4 +++-
 src/journal/journald-context.c | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c
index 39d6799b59..53397e8cbf 100644
--- a/src/core/dbus-unit.c
+++ b/src/core/dbus-unit.c
@@ -1229,7 +1229,9 @@ static int append_process(sd_bus_message *reply, const char *p, pid_t pid, Set *
                 p = buf;
         }
 
-        (void) get_process_cmdline(pid, SIZE_MAX, PROCESS_CMDLINE_COMM_FALLBACK, &cmdline);
+        (void) get_process_cmdline(pid, SIZE_MAX,
+                                   PROCESS_CMDLINE_COMM_FALLBACK | PROCESS_CMDLINE_QUOTE,
+                                   &cmdline);
 
         return sd_bus_message_append(reply,
                                      "(sus)",
diff --git a/src/journal/journald-context.c b/src/journal/journald-context.c
index add0a5480d..694056d558 100644
--- a/src/journal/journald-context.c
+++ b/src/journal/journald-context.c
@@ -225,7 +225,7 @@ static void client_context_read_basic(ClientContext *c) {
         if (get_process_exe(c->pid, &t) >= 0)
                 free_and_replace(c->exe, t);
 
-        if (get_process_cmdline(c->pid, SIZE_MAX, 0, &t) >= 0)
+        if (get_process_cmdline(c->pid, SIZE_MAX, PROCESS_CMDLINE_QUOTE, &t) >= 0)
                 free_and_replace(c->cmdline, t);
 
         if (get_process_capeff(c->pid, &t) >= 0)