From af229d7a5be768a49a7f0d1028d878a71b797bad Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Fri, 16 Feb 2018 06:58:33 +0100
Subject: [PATCH 1/9] login,user-sessions: always warn when we fail to remove
 nologin file

This usually is very annoying to users who then cannot log in, so
make sure we always warn if that happens (selinux, or whatever other reason).

This reverts a790812cb349c5cef95d1b4a20fc80ca08d3a145.
---
 src/basic/fs-util.c               | 11 +++++++++++
 src/basic/fs-util.h               |  2 ++
 src/login/logind-dbus.c           |  2 +-
 src/login/logind.c                |  3 ++-
 src/user-sessions/user-sessions.c | 26 +++++---------------------
 5 files changed, 21 insertions(+), 23 deletions(-)

diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c
index c96c7d0d25..47edcbb04f 100644
--- a/src/basic/fs-util.c
+++ b/src/basic/fs-util.c
@@ -554,6 +554,17 @@ int tmp_dir(const char **ret) {
         return tmp_dir_internal("/tmp", ret);
 }
 
+int unlink_or_warn(const char *filename) {
+        if (unlink(filename) < 0 && errno != ENOENT)
+                /* If the file doesn't exist and the fs simply was read-only (in which
+                 * case unlink() returns EROFS even if the file doesn't exist), don't
+                 * complain */
+                if (errno != EROFS || access(filename, F_OK) >= 0)
+                        return log_error_errno(errno, "Failed to remove \"%s\": %m", filename);
+
+        return 0;
+}
+
 int inotify_add_watch_fd(int fd, int what, uint32_t mask) {
         char path[STRLEN("/proc/self/fd/") + DECIMAL_STR_MAX(int) + 1];
         int r;
diff --git a/src/basic/fs-util.h b/src/basic/fs-util.h
index ae40d6d37f..acb83dfd83 100644
--- a/src/basic/fs-util.h
+++ b/src/basic/fs-util.h
@@ -64,6 +64,8 @@ int get_files_in_directory(const char *path, char ***list);
 int tmp_dir(const char **ret);
 int var_tmp_dir(const char **ret);
 
+int unlink_or_warn(const char *filename);
+
 #define INOTIFY_EVENT_MAX (sizeof(struct inotify_event) + NAME_MAX + 1)
 
 #define FOREACH_INOTIFY_EVENT(e, buffer, sz) \
diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index ed9eba1ee0..aa4bbf7739 100644
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -2010,7 +2010,7 @@ static void reset_scheduled_shutdown(Manager *m) {
         m->shutdown_dry_run = false;
 
         if (m->unlink_nologin) {
-                (void) unlink("/run/nologin");
+                (void) unlink_or_warn("/run/nologin");
                 m->unlink_nologin = false;
         }
 
diff --git a/src/login/logind.c b/src/login/logind.c
index d9b5f026cf..0a869ba447 100644
--- a/src/login/logind.c
+++ b/src/login/logind.c
@@ -35,6 +35,7 @@
 #include "dirent-util.h"
 #include "fd-util.h"
 #include "format-util.h"
+#include "fs-util.h"
 #include "logind.h"
 #include "process-util.h"
 #include "selinux-util.h"
@@ -183,7 +184,7 @@ static void manager_free(Manager *m) {
         udev_unref(m->udev);
 
         if (m->unlink_nologin)
-                (void) unlink("/run/nologin");
+                (void) unlink_or_warn("/run/nologin");
 
         bus_verify_polkit_async_registry_free(m->polkit_registry);
 
diff --git a/src/user-sessions/user-sessions.c b/src/user-sessions/user-sessions.c
index 795766a657..1aed9adae7 100644
--- a/src/user-sessions/user-sessions.c
+++ b/src/user-sessions/user-sessions.c
@@ -23,12 +23,14 @@
 
 #include "fileio.h"
 #include "fileio-label.h"
+#include "fs-util.h"
 #include "log.h"
 #include "selinux-util.h"
 #include "string-util.h"
 #include "util.h"
 
 int main(int argc, char*argv[]) {
+        int r, k;
 
         if (argc != 2) {
                 log_error("This program requires one argument.");
@@ -44,30 +46,12 @@ int main(int argc, char*argv[]) {
         mac_selinux_init();
 
         if (streq(argv[1], "start")) {
-                int r = 0;
-
-                if (unlink("/run/nologin") < 0 && errno != ENOENT)
-                        r = log_error_errno(errno,
-                                            "Failed to remove /run/nologin file: %m");
-
-                if (unlink("/etc/nologin") < 0 && errno != ENOENT) {
-                        /* If the file doesn't exist and /etc simply
-                         * was read-only (in which case unlink()
-                         * returns EROFS even if the file doesn't
-                         * exist), don't complain */
-
-                        if (errno != EROFS || access("/etc/nologin", F_OK) >= 0) {
-                                log_error_errno(errno, "Failed to remove /etc/nologin file: %m");
-                                return EXIT_FAILURE;
-                        }
-                }
-
-                if (r < 0)
+                r = unlink_or_warn("/run/nologin");
+                k = unlink_or_warn("/etc/nologin");
+                if (r < 0 || k < 0)
                         return EXIT_FAILURE;
 
         } else if (streq(argv[1], "stop")) {
-                int r;
-
                 r = write_string_file_atomic_label("/run/nologin", "System is going down.");
                 if (r < 0) {
                         log_error_errno(r, "Failed to create /run/nologin: %m");

From cb51ee7a6e537da5706b4eb4a6d673ebe9654ceb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Fri, 16 Feb 2018 07:00:22 +0100
Subject: [PATCH 2/9] Add some handling to remaining unlinkat calls

Coverity now started warning about this ("Calling unlinkat without checking
return value (as is done elsewhere 12 out of 15 times).", and it is right:
most of the time we should at list print a log message so people can figure
out something is wrong when this happens.

v2:
- use warning level in journald too (this is unlikely to happen ever, so it
  should be safe to something that is visible by default).
---
 src/journal/journald-stream.c | 4 +++-
 src/login/logind.c            | 4 +++-
 src/udev/udev-watch.c         | 2 +-
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/journal/journald-stream.c b/src/journal/journald-stream.c
index 671ada718c..bb7330f3db 100644
--- a/src/journal/journald-stream.c
+++ b/src/journal/journald-stream.c
@@ -778,7 +778,9 @@ int server_restore_streams(Server *s, FDSet *fds) {
                 if (!found) {
                         /* No file descriptor? Then let's delete the state file */
                         log_debug("Cannot restore stream file %s", de->d_name);
-                        unlinkat(dirfd(d), de->d_name, 0);
+                        if (unlinkat(dirfd(d), de->d_name, 0) < 0)
+                                log_warning("Failed to remove /run/systemd/journal/streams/%s: %m",
+                                            de->d_name);
                         continue;
                 }
 
diff --git a/src/login/logind.c b/src/login/logind.c
index 0a869ba447..3fa7d2fed6 100644
--- a/src/login/logind.c
+++ b/src/login/logind.c
@@ -323,7 +323,9 @@ static int manager_enumerate_seats(Manager *m) {
 
                 s = hashmap_get(m->seats, de->d_name);
                 if (!s) {
-                        unlinkat(dirfd(d), de->d_name, 0);
+                        if (unlinkat(dirfd(d), de->d_name, 0) < 0)
+                                log_warning("Failed to remove /run/systemd/seats/%s: %m",
+                                            de->d_name);
                         continue;
                 }
 
diff --git a/src/udev/udev-watch.c b/src/udev/udev-watch.c
index 351cb6345a..61179ce03d 100644
--- a/src/udev/udev-watch.c
+++ b/src/udev/udev-watch.c
@@ -79,7 +79,7 @@ void udev_watch_restore(struct udev *udev) {
                         udev_watch_begin(udev, dev);
                         udev_device_unref(dev);
 unlink:
-                        unlinkat(dirfd(dir), ent->d_name, 0);
+                        (void) unlinkat(dirfd(dir), ent->d_name, 0);
                 }
 
                 closedir(dir);

From 6fb05b079997e99cbd6ffc491c0cfb9abfee7ab1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Fri, 16 Feb 2018 07:03:13 +0100
Subject: [PATCH 3/9] test-process-util: one more (void)

---
 src/test/test-process-util.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/test/test-process-util.c b/src/test/test-process-util.c
index 72edcbb7d6..0e5a9d811d 100644
--- a/src/test/test-process-util.c
+++ b/src/test/test-process-util.c
@@ -100,7 +100,7 @@ static void test_get_process_comm(pid_t pid) {
         if (!detect_container())
                 assert_se(get_ctty_devnr(pid, &h) == -ENXIO || pid != 1);
 
-        getenv_for_pid(pid, "PATH", &i);
+        (void) getenv_for_pid(pid, "PATH", &i);
         log_info("PID"PID_FMT" $PATH: '%s'", pid, strna(i));
 }
 

From b48382e4e0fe5064d73b4916a06424afe0f6c511 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Fri, 16 Feb 2018 09:29:57 +0100
Subject: [PATCH 4/9] shared/conf-parser: define a macro for the repeating
 argument set

The arguments have to be indentical everywhere, so let's use a macro to
make things more readable. But only in the headers, in the .c files let's
keep them verbose so that it's easy to see the argument list.
---
 src/shared/conf-parser.h | 85 ++++++++++++++++++----------------------
 1 file changed, 38 insertions(+), 47 deletions(-)

diff --git a/src/shared/conf-parser.h b/src/shared/conf-parser.h
index 2fd135baa0..8cbb569696 100644
--- a/src/shared/conf-parser.h
+++ b/src/shared/conf-parser.h
@@ -121,44 +121,45 @@ int config_parse_many(
                 void *userdata);
 
 /* Generic parsers */
-int config_parse_int(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
-int config_parse_unsigned(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
-int config_parse_long(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
-int config_parse_uint8(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
-int config_parse_uint16(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
-int config_parse_uint32(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
-int config_parse_uint64(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
-int config_parse_double(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line,  const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
-int config_parse_iec_size(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
-int config_parse_si_size(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
-int config_parse_iec_uint64(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
-int config_parse_bool(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
-int config_parse_tristate(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
-int config_parse_string(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
-int config_parse_path(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
-int config_parse_strv(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
-int config_parse_sec(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
-int config_parse_nsec(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
-int config_parse_mode(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
-int config_parse_log_facility(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
-int config_parse_log_level(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
-int config_parse_signal(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
-int config_parse_personality(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
-int config_parse_ifname(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
-int config_parse_ip_port(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
+#define GENERIC_PARSER_ARGS \
+                const char *unit,                                       \
+                const char *filename,                                   \
+                unsigned line,                                          \
+                const char *section,                                    \
+                unsigned section_line,                                  \
+                const char *lvalue,                                     \
+                int ltype,                                              \
+                const char *rvalue,                                     \
+                void *data,                                             \
+                void *userdata
+int config_parse_int(GENERIC_PARSER_ARGS);
+int config_parse_unsigned(GENERIC_PARSER_ARGS);
+int config_parse_long(GENERIC_PARSER_ARGS);
+int config_parse_uint8(GENERIC_PARSER_ARGS);
+int config_parse_uint16(GENERIC_PARSER_ARGS);
+int config_parse_uint32(GENERIC_PARSER_ARGS);
+int config_parse_uint64(GENERIC_PARSER_ARGS);
+int config_parse_double(GENERIC_PARSER_ARGS);
+int config_parse_iec_size(GENERIC_PARSER_ARGS);
+int config_parse_si_size(GENERIC_PARSER_ARGS);
+int config_parse_iec_uint64(GENERIC_PARSER_ARGS);
+int config_parse_bool(GENERIC_PARSER_ARGS);
+int config_parse_tristate(GENERIC_PARSER_ARGS);
+int config_parse_string(GENERIC_PARSER_ARGS);
+int config_parse_path(GENERIC_PARSER_ARGS);
+int config_parse_strv(GENERIC_PARSER_ARGS);
+int config_parse_sec(GENERIC_PARSER_ARGS);
+int config_parse_nsec(GENERIC_PARSER_ARGS);
+int config_parse_mode(GENERIC_PARSER_ARGS);
+int config_parse_log_facility(GENERIC_PARSER_ARGS);
+int config_parse_log_level(GENERIC_PARSER_ARGS);
+int config_parse_signal(GENERIC_PARSER_ARGS);
+int config_parse_personality(GENERIC_PARSER_ARGS);
+int config_parse_ifname(GENERIC_PARSER_ARGS);
+int config_parse_ip_port(GENERIC_PARSER_ARGS);
 
 #define DEFINE_CONFIG_PARSE_ENUM(function,name,type,msg)                \
-        int function(const char *unit,                                  \
-                     const char *filename,                              \
-                     unsigned line,                                     \
-                     const char *section,                               \
-                     unsigned section_line,                             \
-                     const char *lvalue,                                \
-                     int ltype,                                         \
-                     const char *rvalue,                                \
-                     void *data,                                        \
-                     void *userdata) {                                  \
-                                                                        \
+        int function(GENERIC_PARSER_ARGS) {                             \
                 type *i = data, x;                                      \
                                                                         \
                 assert(filename);                                       \
@@ -177,17 +178,7 @@ int config_parse_ip_port(const char *unit, const char *filename, unsigned line,
         }
 
 #define DEFINE_CONFIG_PARSE_ENUMV(function,name,type,invalid,msg)              \
-        int function(const char *unit,                                         \
-                     const char *filename,                                     \
-                     unsigned line,                                            \
-                     const char *section,                                      \
-                     unsigned section_line,                                    \
-                     const char *lvalue,                                       \
-                     int ltype,                                                \
-                     const char *rvalue,                                       \
-                     void *data,                                               \
-                     void *userdata) {                                         \
-                                                                               \
+        int function(GENERIC_PARSER_ARGS) {                                    \
                 type **enums = data, x, *ys;                                   \
                 _cleanup_free_ type *xs = NULL;                                \
                 const char *word, *state;                                      \

From 9ecdba8cb75dd9c529ad3a869b25c36752fad90c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Fri, 16 Feb 2018 09:56:29 +0100
Subject: [PATCH 5/9] Move config_parse_join_controllers to shared, add test

config_parse_join_controllers would free the destination argument on failure,
which is contrary to our normal style, where failed parsing has no effect.
Moving it to shared also allows a test to be added.
---
 src/basic/strv.h            |   1 +
 src/core/main.c             | 101 ----------------------------------
 src/core/manager.c          |   1 -
 src/shared/conf-parser.c    | 105 ++++++++++++++++++++++++++++++++++++
 src/shared/conf-parser.h    |   1 +
 src/test/test-conf-parser.c |  38 +++++++++++++
 6 files changed, 145 insertions(+), 102 deletions(-)

diff --git a/src/basic/strv.h b/src/basic/strv.h
index 44fe1f279c..f169ac5d4f 100644
--- a/src/basic/strv.h
+++ b/src/basic/strv.h
@@ -179,6 +179,7 @@ static inline bool strv_fnmatch_or_empty(char* const* patterns, const char *s, i
 }
 
 char ***strv_free_free(char ***l);
+DEFINE_TRIVIAL_CLEANUP_FUNC(char***, strv_free_free);
 
 char **strv_skip(char **l, size_t n);
 
diff --git a/src/core/main.c b/src/core/main.c
index 56200a8fad..44ebf4f1da 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -652,107 +652,6 @@ static int config_parse_crash_chvt(
         return 0;
 }
 
-static int config_parse_join_controllers(const char *unit,
-                                         const char *filename,
-                                         unsigned line,
-                                         const char *section,
-                                         unsigned section_line,
-                                         const char *lvalue,
-                                         int ltype,
-                                         const char *rvalue,
-                                         void *data,
-                                         void *userdata) {
-
-        const char *whole_rvalue = rvalue;
-        unsigned n = 0;
-
-        assert(filename);
-        assert(lvalue);
-        assert(rvalue);
-
-        arg_join_controllers = strv_free_free(arg_join_controllers);
-
-        for (;;) {
-                _cleanup_free_ char *word = NULL;
-                char **l;
-                int r;
-
-                r = extract_first_word(&rvalue, &word, NULL, EXTRACT_QUOTES);
-                if (r < 0) {
-                        log_syntax(unit, LOG_ERR, filename, line, r, "Invalid value for %s: %s", lvalue, whole_rvalue);
-                        return r;
-                }
-                if (r == 0)
-                        break;
-
-                l = strv_split(word, ",");
-                if (!l)
-                        return log_oom();
-                strv_uniq(l);
-
-                if (strv_length(l) <= 1) {
-                        strv_free(l);
-                        continue;
-                }
-
-                if (!arg_join_controllers) {
-                        arg_join_controllers = new(char**, 2);
-                        if (!arg_join_controllers) {
-                                strv_free(l);
-                                return log_oom();
-                        }
-
-                        arg_join_controllers[0] = l;
-                        arg_join_controllers[1] = NULL;
-
-                        n = 1;
-                } else {
-                        char ***a;
-                        char ***t;
-
-                        t = new0(char**, n+2);
-                        if (!t) {
-                                strv_free(l);
-                                return log_oom();
-                        }
-
-                        n = 0;
-
-                        for (a = arg_join_controllers; *a; a++) {
-
-                                if (strv_overlap(*a, l)) {
-                                        if (strv_extend_strv(&l, *a, false) < 0) {
-                                                strv_free(l);
-                                                strv_free_free(t);
-                                                return log_oom();
-                                        }
-
-                                } else {
-                                        char **c;
-
-                                        c = strv_copy(*a);
-                                        if (!c) {
-                                                strv_free(l);
-                                                strv_free_free(t);
-                                                return log_oom();
-                                        }
-
-                                        t[n++] = c;
-                                }
-                        }
-
-                        t[n++] = strv_uniq(l);
-
-                        strv_free_free(arg_join_controllers);
-                        arg_join_controllers = t;
-                }
-        }
-        if (!isempty(rvalue))
-                log_syntax(unit, LOG_ERR, filename, line, 0, "Trailing garbage, ignoring.");
-
-        return 0;
-}
-
 static int parse_config_file(void) {
 
         const ConfigTableItem items[] = {
diff --git a/src/core/manager.c b/src/core/manager.c
index e4ef461ac7..d22c0e1d42 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -2710,7 +2710,6 @@ void manager_send_unit_plymouth(Manager *m, Unit *u) {
         }
 
         if (connect(fd, &sa.sa, SOCKADDR_UN_LEN(sa.un)) < 0) {
-
                 if (!IN_SET(errno, EPIPE, EAGAIN, ENOENT, ECONNREFUSED, ECONNRESET, ECONNABORTED))
                         log_error_errno(errno, "connect() failed: %m");
                 return;
diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c
index daddb7cf20..5c033e0ec0 100644
--- a/src/shared/conf-parser.c
+++ b/src/shared/conf-parser.c
@@ -1021,3 +1021,108 @@ int config_parse_ip_port(
 
         return 0;
 }
+
+int config_parse_join_controllers(
+                const char *unit,
+                const char *filename,
+                unsigned line,
+                const char *section,
+                unsigned section_line,
+                const char *lvalue,
+                int ltype,
+                const char *rvalue,
+                void *data,
+                void *userdata) {
+
+        char ****ret = data;
+        const char *whole_rvalue = rvalue;
+        unsigned n = 0;
+        _cleanup_(strv_free_freep) char ***controllers = NULL;
+
+        assert(filename);
+        assert(lvalue);
+        assert(rvalue);
+        assert(ret);
+
+        for (;;) {
+                _cleanup_free_ char *word = NULL;
+                char **l;
+                int r;
+
+                r = extract_first_word(&rvalue, &word, NULL, EXTRACT_QUOTES);
+                if (r < 0) {
+                        log_syntax(unit, LOG_ERR, filename, line, r, "Invalid value for %s: %s", lvalue, whole_rvalue);
+                        return r;
+                }
+                if (r == 0)
+                        break;
+
+                l = strv_split(word, ",");
+                if (!l)
+                        return log_oom();
+                strv_uniq(l);
+
+                if (strv_length(l) <= 1) {
+                        strv_free(l);
+                        continue;
+                }
+
+                if (!controllers) {
+                        controllers = new(char**, 2);
+                        if (!controllers) {
+                                strv_free(l);
+                                return log_oom();
+                        }
+
+                        controllers[0] = l;
+                        controllers[1] = NULL;
+
+                        n = 1;
+                } else {
+                        char ***a;
+                        char ***t;
+
+                        t = new0(char**, n+2);
+                        if (!t) {
+                                strv_free(l);
+                                return log_oom();
+                        }
+
+                        n = 0;
+
+                        for (a = controllers; *a; a++)
+                                if (strv_overlap(*a, l)) {
+                                        if (strv_extend_strv(&l, *a, false) < 0) {
+                                                strv_free(l);
+                                                strv_free_free(t);
+                                                return log_oom();
+                                        }
+
+                                } else {
+                                        char **c;
+
+                                        c = strv_copy(*a);
+                                        if (!c) {
+                                                strv_free(l);
+                                                strv_free_free(t);
+                                                return log_oom();
+                                        }
+
+                                        t[n++] = c;
+                                }
+
+                        t[n++] = strv_uniq(l);
+
+                        strv_free_free(controllers);
+                        controllers = t;
+                }
+        }
+        if (!isempty(rvalue))
+                log_syntax(unit, LOG_ERR, filename, line, 0, "Trailing garbage, ignoring.");
+
+        strv_free_free(*ret);
+        *ret = controllers;
+        controllers = NULL;
+
+        return 0;
+}
diff --git a/src/shared/conf-parser.h b/src/shared/conf-parser.h
index 8cbb569696..908f530713 100644
--- a/src/shared/conf-parser.h
+++ b/src/shared/conf-parser.h
@@ -157,6 +157,7 @@ int config_parse_signal(GENERIC_PARSER_ARGS);
 int config_parse_personality(GENERIC_PARSER_ARGS);
 int config_parse_ifname(GENERIC_PARSER_ARGS);
 int config_parse_ip_port(GENERIC_PARSER_ARGS);
+int config_parse_join_controllers(GENERIC_PARSER_ARGS);
 
 #define DEFINE_CONFIG_PARSE_ENUM(function,name,type,msg)                \
         int function(GENERIC_PARSER_ARGS) {                             \
diff --git a/src/test/test-conf-parser.c b/src/test/test-conf-parser.c
index 770bb9600f..84789e3bb8 100644
--- a/src/test/test-conf-parser.c
+++ b/src/test/test-conf-parser.c
@@ -226,6 +226,43 @@ static void test_config_parse_iec_uint64(void) {
         assert_se(config_parse_iec_uint64(NULL, "/this/file", 11, "Section", 22, "Size", 0, "4.5M", &offset, NULL) == 0);
 }
 
+static void test_config_parse_join_controllers(void) {
+        int r;
+        _cleanup_(strv_free_freep) char ***c = NULL;
+        char ***c2;
+
+        /* Test normal operation */
+        r = config_parse_join_controllers(NULL, "example.conf", 11, "Section", 10, "JoinControllers", 0, "cpu,cpuacct net_cls,netprio", &c, NULL);
+        assert_se(r == 0);
+        assert_se(c);
+        assert_se(strv_length(c[0]) == 2);
+        assert_se(strv_equal(c[0], STRV_MAKE("cpu", "cpuacct")));
+        assert_se(strv_length(c[1]) == 2);
+        assert_se(strv_equal(c[1], STRV_MAKE("net_cls", "netprio")));
+        assert_se(c[2] == NULL);
+
+        /* Test special case of no mounted controllers */
+        r = config_parse_join_controllers(NULL, "example.conf", 12, "Section", 10, "JoinControllers", 0, "", &c, NULL);
+        assert_se(r == 0);
+        assert_se(c == NULL);
+
+        /* Test merging of overlapping lists */
+        r = config_parse_join_controllers(NULL, "example.conf", 13, "Section", 10, "JoinControllers", 0, "a,b b,c", &c, NULL);
+        assert_se(r == 0);
+        assert_se(c);
+        assert_se(strv_length(c[0]) == 3);
+        assert_se(strv_contains(c[0], "a"));
+        assert_se(strv_contains(c[0], "b"));
+        assert_se(strv_contains(c[0], "c"));
+        assert_se(c[1] == NULL);
+
+        /* Test ignoring of bad lines */
+        c2 = c;
+        r = config_parse_join_controllers(NULL, "example.conf", 14, "Section", 10, "JoinControllers", 0, "a,\"b ", &c, NULL);
+        assert_se(r < 0);
+        assert_se(c == c2);
+}
+
 #define x10(x) x x x x x x x x x x
 #define x100(x) x10(x10(x))
 #define x1000(x) x10(x100(x))
@@ -365,6 +402,7 @@ int main(int argc, char **argv) {
         test_config_parse_sec();
         test_config_parse_nsec();
         test_config_parse_iec_uint64();
+        test_config_parse_join_controllers();
 
         for (i = 0; i < ELEMENTSOF(config_file); i++)
                 test_config_parse(i, config_file[i]);

From 56c8d7444ac212c48443a0d30e42ea279140f96d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Fri, 16 Feb 2018 10:17:46 +0100
Subject: [PATCH 6/9] pid1: do not initialize join_controllers by default

We're moving towards unified cgroup hierarchy where this is not necessary.
This makes main.c a bit simpler.
---
 src/core/main.c             | 32 --------------------------------
 src/core/mount-setup.c      | 20 ++++++++++++++++----
 src/shared/conf-parser.c    | 11 +++++++++++
 src/test/test-conf-parser.c |  4 +++-
 4 files changed, 30 insertions(+), 37 deletions(-)

diff --git a/src/core/main.c b/src/core/main.c
index 44ebf4f1da..bdb7bbf221 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -1213,32 +1213,6 @@ static void test_usr(void) {
                     "Consult http://freedesktop.org/wiki/Software/systemd/separate-usr-is-broken for more information.");
 }
 
-static int initialize_join_controllers(void) {
-        /* By default, mount "cpu" + "cpuacct" together, and "net_cls"
-         * + "net_prio". We'd like to add "cpuset" to the mix, but
-         * "cpuset" doesn't really work for groups with no initialized
-         * attributes. */
-
-        arg_join_controllers = new(char**, 3);
-        if (!arg_join_controllers)
-                return -ENOMEM;
-
-        arg_join_controllers[0] = strv_new("cpu", "cpuacct", NULL);
-        if (!arg_join_controllers[0])
-                goto oom;
-
-        arg_join_controllers[1] = strv_new("net_cls", "net_prio", NULL);
-        if (!arg_join_controllers[1])
-                goto oom;
-
-        arg_join_controllers[2] = NULL;
-        return 0;
-
-oom:
-        arg_join_controllers = strv_free_free(arg_join_controllers);
-        return -ENOMEM;
-}
-
 static int enforce_syscall_archs(Set *archs) {
 #if HAVE_SECCOMP
         int r;
@@ -1993,12 +1967,6 @@ static int load_configuration(int argc, char **argv, const char **ret_error_mess
 
         assert(ret_error_message);
 
-        r = initialize_join_controllers();
-        if (r < 0) {
-                *ret_error_message = "Failed to initialize cgroup controller joining table";
-                return r;
-        }
-
         arg_default_tasks_max = system_tasks_max_scale(DEFAULT_TASKS_MAX_PERCENTAGE, 100U);
 
         r = parse_config_file();
diff --git a/src/core/mount-setup.c b/src/core/mount-setup.c
index a0c5f5aaae..ed78c945d1 100644
--- a/src/core/mount-setup.c
+++ b/src/core/mount-setup.c
@@ -253,6 +253,19 @@ int mount_cgroup_controllers(char ***join_controllers) {
 
         /* Mount all available cgroup controllers that are built into the kernel. */
 
+        if (!join_controllers)
+                /* The defaults:
+                 * mount "cpu" + "cpuacct" together, and "net_cls" + "net_prio".
+                 *
+                 * We'd like to add "cpuset" to the mix, but "cpuset" doesn't really
+                 * work for groups with no initialized attributes.
+                 */
+                join_controllers = (char**[]) {
+                        STRV_MAKE("cpu", "cpuacct"),
+                        STRV_MAKE("net_cls", "net_prio"),
+                        NULL,
+                };
+
         r = cg_kernel_controllers(&controllers);
         if (r < 0)
                 return log_error_errno(r, "Failed to enumerate cgroup controllers: %m");
@@ -271,10 +284,9 @@ int mount_cgroup_controllers(char ***join_controllers) {
                 if (!controller)
                         break;
 
-                if (join_controllers)
-                        for (k = join_controllers; *k; k++)
-                                if (strv_find(*k, controller))
-                                        break;
+                for (k = join_controllers; *k; k++)
+                        if (strv_find(*k, controller))
+                                break;
 
                 if (k && *k) {
                         char **i, **j;
diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c
index 5c033e0ec0..15cfe4e4f7 100644
--- a/src/shared/conf-parser.c
+++ b/src/shared/conf-parser.c
@@ -1120,6 +1120,17 @@ int config_parse_join_controllers(
         if (!isempty(rvalue))
                 log_syntax(unit, LOG_ERR, filename, line, 0, "Trailing garbage, ignoring.");
 
+        /* As a special case, return a single empty strv, to override the default */
+        if (!controllers) {
+                controllers = new(char**, 2);
+                if (!controllers)
+                        return log_oom();
+                controllers[0] = strv_new(NULL, NULL);
+                if (!controllers[0])
+                        return log_oom();
+                controllers[1] = NULL;
+        }
+
         strv_free_free(*ret);
         *ret = controllers;
         controllers = NULL;
diff --git a/src/test/test-conf-parser.c b/src/test/test-conf-parser.c
index 84789e3bb8..72c53ab6a0 100644
--- a/src/test/test-conf-parser.c
+++ b/src/test/test-conf-parser.c
@@ -244,7 +244,9 @@ static void test_config_parse_join_controllers(void) {
         /* Test special case of no mounted controllers */
         r = config_parse_join_controllers(NULL, "example.conf", 12, "Section", 10, "JoinControllers", 0, "", &c, NULL);
         assert_se(r == 0);
-        assert_se(c == NULL);
+        assert_se(c);
+        assert_se(strv_equal(c[0], STRV_MAKE_EMPTY));
+        assert_se(c[1] == NULL);
 
         /* Test merging of overlapping lists */
         r = config_parse_join_controllers(NULL, "example.conf", 13, "Section", 10, "JoinControllers", 0, "a,b b,c", &c, NULL);

From f4aa0bde1cdb2a975be2ca2be4c68eb982d62460 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Fri, 16 Feb 2018 10:23:16 +0100
Subject: [PATCH 7/9] core: drop obsolete comment

https://github.com/systemd/systemd/pull/8125#pullrequestreview-96894581
---
 src/core/unit.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/core/unit.c b/src/core/unit.c
index bd077cb359..c308950932 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -2458,7 +2458,6 @@ void unit_notify(Unit *u, UnitActiveState os, UnitActiveState ns, bool reload_su
                 }
         }
 
-        /* Some names are special */
         if (UNIT_IS_ACTIVE_OR_RELOADING(ns)) {
 
                 if (u->type == UNIT_SERVICE &&

From f95dbcc27b02396d1ca865cfd5effa7b205e762a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Fri, 16 Feb 2018 10:27:47 +0100
Subject: [PATCH 8/9] Some comment adjustments

Follow up for review of #8184.
---
 src/basic/terminal-util.c     | 12 ++++++------
 src/shared/ask-password-api.c |  2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c
index a897a6367d..cddbb461bd 100644
--- a/src/basic/terminal-util.c
+++ b/src/basic/terminal-util.c
@@ -387,8 +387,8 @@ int acquire_terminal(
          *
          * Note: strictly speaking this actually watches for the device being closed, it does *not* really watch
          * whether a tty loses its controlling process. However, unless some rogue process uses TIOCNOTTY on /dev/tty
-         * *after* closing its tty otherwise this will not become a problem. As long as the administrator makes sure
-         * not configure any service on the same tty as an untrusted user this should not be a problem. (Which he
+         * *after* closing its tty otherwise this will not become a problem. As long as the administrator makes sure to
+         * not configure any service on the same tty as an untrusted user this should not be a problem. (Which they
          * probably should not do anyway.) */
 
         if ((flags & ~ACQUIRE_TERMINAL_PERMISSIVE) == ACQUIRE_TERMINAL_WAIT) {
@@ -731,7 +731,7 @@ int get_kernel_consoles(char ***ret) {
 
         assert(ret);
 
-        /* If we /sys is mounted read-only this means we are running in some kind of container environment. In that
+        /* If /sys is mounted read-only this means we are running in some kind of container environment. In that
          * case /sys would reflect the host system, not us, hence ignore the data we can read from it. */
         if (path_is_read_only_fs("/sys") > 0)
                 goto fallback;
@@ -1242,7 +1242,7 @@ bool terminal_is_dumb(void) {
 bool colors_enabled(void) {
 
         /* Returns true if colors are considered supported on our stdout. For that we check $SYSTEMD_COLORS first
-         * (which is the explicit way to turn off/on colors). If that didn't work we turn off colors unless we are on a
+         * (which is the explicit way to turn colors on/off). If that didn't work we turn colors off unless we are on a
          * TTY. And if we are on a TTY we turn it off if $TERM is set to "dumb". There's one special tweak though: if
          * we are PID 1 then we do not check whether we are connected to a TTY, because we don't keep /dev/console open
          * continously due to fear of SAK, and hence things are a bit weird. */
@@ -1270,8 +1270,8 @@ bool dev_console_colors_enabled(void) {
         /* Returns true if we assume that color is supported on /dev/console.
          *
          * For that we first check if we explicitly got told to use colors or not, by checking $SYSTEMD_COLORS. If that
-         * didn't tell us anything we check whether PID 1 has $TERM set, and if not whether $TERM is set on the kernel
-         * command line. If we find $TERM set we assume color if it's not set to "dumb", similar to regular
+         * isn't set we check whether PID 1 has $TERM set, and if not, whether TERM is set on the kernel command
+         * line. If we find $TERM set we assume color if it's not set to "dumb", similarly to how regular
          * colors_enabled() operates. */
 
         b = getenv_bool("SYSTEMD_COLORS");
diff --git a/src/shared/ask-password-api.c b/src/shared/ask-password-api.c
index a0ee05e2d8..4fa9188957 100644
--- a/src/shared/ask-password-api.c
+++ b/src/shared/ask-password-api.c
@@ -209,7 +209,7 @@ static void backspace_string(int ttyfd, const char *str) {
         if (ttyfd < 0)
                 return;
 
-        /* Backspaces back for enough characters to entirely undo printing of the specified string. */
+        /* Backspaces through enough characters to entirely undo printing of the specified string. */
 
         m = utf8_n_codepoints(str);
         if (m == (size_t) -1)

From e2cbc803f02a7adac7fe8615a0f2b88e12a3a303 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Fri, 16 Feb 2018 10:34:00 +0100
Subject: [PATCH 9/9] tests: add a simple test for utf8_n_codepoints()

---
 src/test/test-utf8.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/src/test/test-utf8.c b/src/test/test-utf8.c
index 1ea0901f69..c04c99dcad 100644
--- a/src/test/test-utf8.c
+++ b/src/test/test-utf8.c
@@ -106,6 +106,15 @@ static void test_utf16_to_utf8(void) {
         free(a);
 }
 
+static void test_utf8_n_codepoints(void) {
+        assert_se(utf8_n_codepoints("abc") == 3);
+        assert_se(utf8_n_codepoints("zażółcić gęślą jaźń") == 19);
+        assert_se(utf8_n_codepoints("串") == 1);
+        assert_se(utf8_n_codepoints("") == 0);
+        assert_se(utf8_n_codepoints("…👊🔪💐…") == 5);
+        assert_se(utf8_n_codepoints("\xF1") == (size_t) -1);
+}
+
 int main(int argc, char *argv[]) {
         test_utf8_is_valid();
         test_utf8_is_printable();
@@ -114,6 +123,7 @@ int main(int argc, char *argv[]) {
         test_utf8_escaping();
         test_utf8_escaping_printable();
         test_utf16_to_utf8();
+        test_utf8_n_codepoints();
 
         return 0;
 }