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

Lennart Poettering lennart at poettering.net
Wed Jun 10 16:47:02 PDT 2015


On Mon, 01.06.15 13:56, Dimitri John Ledkov (dimitri.j.ledkov at intel.com) wrote:

> 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.

For the sake of the archives: this has now been merged as:

https://github.com/systemd/systemd/pull/6

> ---
>  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
> 
> _______________________________________________
> systemd-devel mailing list
> systemd-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Lennart

-- 
Lennart Poettering, Red Hat


More information about the systemd-devel mailing list