[systemd-devel] [PATCH] shared: Drop 'name=' prefix from SYSTEMD_CGROUP_CONTROLLER define.

[Dimitri John Ledkov]

In cgtop,mount-setup,nspawn the name= prefix is hard-coded in the
mount options, and the define is not used.

Everywhere else, we explicitly white-list allow 'name=' prefix to be
used with all controllers, and strip it out to 'normalise' the
controller name. That work is mostly inflicted on us due to 'name='
prefix in the define. Dropping this prefix makes everything more sane
overall.
---
 src/shared/cgroup-util.c    | 24 ++++++++++--------------
 src/shared/cgroup-util.h    |  2 +-
 src/shared/def.h            |  2 +-
 src/test/test-cgroup-util.c | 20 ++++++++++----------
 4 files changed, 22 insertions(+), 26 deletions(-)

diff --git a/src/shared/cgroup-util.c b/src/shared/cgroup-util.c
index 9988e5c..d83cdf7 100644
--- a/src/shared/cgroup-util.c
+++ b/src/shared/cgroup-util.c
@@ -441,9 +441,7 @@ static const char *normalize_controller(const char *controller) {
 
         assert(controller);
 
-        if (streq(controller, SYSTEMD_CGROUP_CONTROLLER))
-                return "systemd";
-        else if (startswith(controller, "name="))
+        if (startswith(controller, "name="))
                 return controller + 5;
         else
                 return controller;
@@ -483,7 +481,7 @@ int cg_get_path(const char *controller, const char *path, const char *suffix, ch
 
         assert(fs);
 
-        if (controller && !cg_controller_is_valid(controller, true))
+        if (controller && !cg_controller_is_valid(controller))
                 return -EINVAL;
 
         if (_unlikely_(!good)) {
@@ -526,7 +524,7 @@ int cg_get_path_and_check(const char *controller, const char *path, const char *
 
         assert(fs);
 
-        if (!cg_controller_is_valid(controller, true))
+        if (!cg_controller_is_valid(controller))
                 return -EINVAL;
 
         /* Normalize the controller syntax */
@@ -742,7 +740,7 @@ int cg_pid_get_path(const char *controller, pid_t pid, char **path) {
         assert(pid >= 0);
 
         if (controller) {
-                if (!cg_controller_is_valid(controller, true))
+                if (!cg_controller_is_valid(controller))
                         return -EINVAL;
 
                 controller = normalize_controller(controller);
@@ -971,7 +969,7 @@ int cg_split_spec(const char *spec, char **controller, char **path) {
 
         e = strchr(spec, ':');
         if (!e) {
-                if (!cg_controller_is_valid(spec, true))
+                if (!cg_controller_is_valid(spec))
                         return -EINVAL;
 
                 if (controller) {
@@ -994,7 +992,7 @@ int cg_split_spec(const char *spec, char **controller, char **path) {
         t = strdup(normalize_controller(v));
         if (!t)
                 return -ENOMEM;
-        if (!cg_controller_is_valid(t, true)) {
+        if (!cg_controller_is_valid(t)) {
                 free(t);
                 return -EINVAL;
         }
@@ -1610,17 +1608,15 @@ char *cg_unescape(const char *p) {
         DIGITS LETTERS                          \
         "_"
 
-bool cg_controller_is_valid(const char *p, bool allow_named) {
+bool cg_controller_is_valid(const char *p) {
         const char *t, *s;
 
         if (!p)
                 return false;
 
-        if (allow_named) {
-                s = startswith(p, "name=");
-                if (s)
-                        p = s;
-        }
+        s = startswith(p, "name=");
+        if (s)
+                p = s;
 
         if (*p == 0 || *p == '_')
                 return false;
diff --git a/src/shared/cgroup-util.h b/src/shared/cgroup-util.h
index cbf7201..fd72e9e 100644
--- a/src/shared/cgroup-util.h
+++ b/src/shared/cgroup-util.h
@@ -122,7 +122,7 @@ int cg_path_decode_unit(const char *cgroup, char **unit);
 char *cg_escape(const char *p);
 char *cg_unescape(const char *p) _pure_;
 
-bool cg_controller_is_valid(const char *p, bool allow_named);
+bool cg_controller_is_valid(const char *p);
 
 int cg_slice_to_path(const char *unit, char **ret);
 
diff --git a/src/shared/def.h b/src/shared/def.h
index a3d9fcf..011c7c6 100644
--- a/src/shared/def.h
+++ b/src/shared/def.h
@@ -35,7 +35,7 @@
  * the watchdog pings will keep the loop busy. */
 #define DEFAULT_EXIT_USEC (30*USEC_PER_SEC)
 
-#define SYSTEMD_CGROUP_CONTROLLER "name=systemd"
+#define SYSTEMD_CGROUP_CONTROLLER "systemd"
 
 #define SIGNALS_CRASH_HANDLER SIGSEGV,SIGILL,SIGFPE,SIGBUS,SIGQUIT,SIGABRT
 #define SIGNALS_IGNORE SIGPIPE
diff --git a/src/test/test-cgroup-util.c b/src/test/test-cgroup-util.c
index 4a89f64..ecc9d70 100644
--- a/src/test/test-cgroup-util.c
+++ b/src/test/test-cgroup-util.c
@@ -244,16 +244,16 @@ static void test_escape(void) {
 }
 
 static void test_controller_is_valid(void) {
-        assert_se(cg_controller_is_valid("foobar", false));
-        assert_se(cg_controller_is_valid("foo_bar", false));
-        assert_se(cg_controller_is_valid("name=foo", true));
-        assert_se(!cg_controller_is_valid("", false));
-        assert_se(!cg_controller_is_valid("name=", true));
-        assert_se(!cg_controller_is_valid("=", false));
-        assert_se(!cg_controller_is_valid("cpu,cpuacct", false));
-        assert_se(!cg_controller_is_valid("_", false));
-        assert_se(!cg_controller_is_valid("_foobar", false));
-        assert_se(!cg_controller_is_valid("tatü", false));
+        assert_se(cg_controller_is_valid("foobar"));
+        assert_se(cg_controller_is_valid("foo_bar"));
+        assert_se(cg_controller_is_valid("name=foo"));
+        assert_se(!cg_controller_is_valid(""));
+        assert_se(!cg_controller_is_valid("name="));
+        assert_se(!cg_controller_is_valid("="));
+        assert_se(!cg_controller_is_valid("cpu,cpuacct"));
+        assert_se(!cg_controller_is_valid("_"));
+        assert_se(!cg_controller_is_valid("_foobar"));
+        assert_se(!cg_controller_is_valid("tatü"));
 }
 
 static void test_slice_to_path_one(const char *unit, const char *path, int error) {
-- 
2.1.4