cgroup-util: rework cg_get_keyed_attribute() a bit

Let's make sure we don't clobber the return parameter on failure, to
follow our coding style. Also, break the loop early if we have all
attributes we need.

This also changes the keys parameter to a simple char**, so that we can
use STRV_MAKE() for passing the list of attributes to read.

This also makes it possible to distuingish the case when the whole
attribute file doesn't exist from one key in it missing. In the former
case we return -ENOENT, in the latter we now return -ENXIO.

4 files changed, 76 insertions, 27 deletions

diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c
index 7c0ba92110..7944785bbf 100644
--- a/src/basic/cgroup-util.c
+++ b/src/basic/cgroup-util.c
@@ -2032,46 +2032,95 @@ int cg_get_attribute(const char *controller, const char *path, const char *attri
         return read_one_line_file(p, ret);
 }
 
-int cg_get_keyed_attribute(const char *controller, const char *path, const char *attribute, const char **keys, char **values) {
-        _cleanup_free_ char *filename = NULL, *content = NULL;
-        char *line, *p;
-        int i, r;
+int cg_get_keyed_attribute(
+                const char *controller,
+                const char *path,
+                const char *attribute,
+                char **keys,
+                char **ret_values) {
 
-        for (i = 0; keys[i]; i++)
-                values[i] = NULL;
+        _cleanup_free_ char *filename = NULL, *contents = NULL;
+        _cleanup_fclose_ FILE *f = NULL;
+        const char *p;
+        size_t n, i;
+        char **v;
+        int r;
+
+        /* Reads one or more fields of a cgroupsv2 keyed attribute file. The 'keys' parameter should be an strv with
+         * all keys to retrieve. The 'ret_values' parameter should be passed as string size with the same number of
+         * entries as 'keys'. On success each entry will be set to the value of the matching key.
+         *
+         * If the attribute file doesn't exist at all returns ENOENT, if any key is not found returns ENXIO. */
 
         r = cg_get_path(controller, path, attribute, &filename);
         if (r < 0)
                 return r;
 
-        r = read_full_file(filename, &content, NULL);
+        r = read_full_file(filename, &contents, NULL);
         if (r < 0)
                 return r;
 
-        p = content;
-        while ((line = strsep(&p, "\n"))) {
-                char *key;
+        n = strv_length(keys);
+        if (n == 0) /* No keys to retrieve? That's easy, we are done then */
+                return 0;
 
-                key = strsep(&line, " ");
+        /* Let's build this up in a temporary array for now in order not to clobber the return parameter on failure */
+        v = newa0(char*, n);
 
-                for (i = 0; keys[i]; i++) {
-                        if (streq(key, keys[i])) {
-                                values[i] = strdup(line);
-                                break;
+        for (p = contents; *p;) {
+                const char *w = NULL;
+                size_t n_done = 0;
+
+                for (i = 0; i < n; i++) {
+                        if (v[i])
+                                n_done ++;
+                        else {
+                                w = first_word(p, keys[i]);
+                                if (w)
+                                        break;
                         }
                 }
-        }
 
-        for (i = 0; keys[i]; i++) {
-                if (!values[i]) {
-                        for (i = 0; keys[i]; i++) {
-                                values[i] = mfree(values[i]);
+                if (w) {
+                        char *c;
+                        size_t l;
+
+                        l = strcspn(w, NEWLINE);
+                        c = strndup(w, l);
+                        if (!c) {
+                                r = -ENOMEM;
+                                goto fail;
                         }
-                        return -ENOENT;
+
+                        v[i] = c;
+                        n_done++;
+
+                        if (n_done >= n)
+                                goto done;
+
+                        p = w + l;
+                } else {
+                        if (n_done >= n)
+                                goto done;
+
+                        p += strcspn(p, NEWLINE);
                 }
+
+                p += strspn(p, NEWLINE);
         }
 
+        r = -ENXIO;
+
+fail:
+        for (i = 0; i < n; i++)
+                free(v[i]);
+
+        return r;
+
+done:
+        memcpy(ret_values, v, sizeof(char*) * n);
         return 0;
+
 }
 
 int cg_create_everywhere(CGroupMask supported, CGroupMask mask, const char *path) {
diff --git a/src/basic/cgroup-util.h b/src/basic/cgroup-util.h
index 05c9f84505..068df102f7 100644
--- a/src/basic/cgroup-util.h
+++ b/src/basic/cgroup-util.h
@@ -186,7 +186,7 @@ int cg_create_and_attach(const char *controller, const char *path, pid_t pid);
 
 int cg_set_attribute(const char *controller, const char *path, const char *attribute, const char *value);
 int cg_get_attribute(const char *controller, const char *path, const char *attribute, char **ret);
-int cg_get_keyed_attribute(const char *controller, const char *path, const char *attribute, const char **keys, char **values);
+int cg_get_keyed_attribute(const char *controller, const char *path, const char *attribute, char **keys, char **values);
 
 int cg_set_access(const char *controller, const char *path, uid_t uid, gid_t gid);
 
diff --git a/src/cgtop/cgtop.c b/src/cgtop/cgtop.c
index ccd5fef0af..d7708dab2d 100644
--- a/src/cgtop/cgtop.c
+++ b/src/cgtop/cgtop.c
@@ -257,14 +257,13 @@ static int process(
                         if (r < 0)
                                 return r;
                 } else if (all_unified) {
-                        const char *keys[] = { "usage_usec", NULL };
                         _cleanup_free_ char *val = NULL;
 
                         if (!streq(controller, "cpu"))
                                 return 0;
 
-                        r = cg_get_keyed_attribute("cpu", path, "cpu.stat", keys, &val);
-                        if (r == -ENOENT)
+                        r = cg_get_keyed_attribute("cpu", path, "cpu.stat", STRV_MAKE("usage_usec"), &val);
+                        if (IN_SET(r, -ENOENT, -ENXIO))
                                 return 0;
                         if (r < 0)
                                 return r;
diff --git a/src/core/cgroup.c b/src/core/cgroup.c
index 97b3756567..8b760a9888 100644
--- a/src/core/cgroup.c
+++ b/src/core/cgroup.c
@@ -2358,16 +2358,17 @@ static int unit_get_cpu_usage_raw(Unit *u, nsec_t *ret) {
         if (r < 0)
                 return r;
         if (r > 0) {
-                const char *keys[] = { "usage_usec", NULL };
                 _cleanup_free_ char *val = NULL;
                 uint64_t us;
 
                 if ((u->cgroup_realized_mask & CGROUP_MASK_CPU) == 0)
                         return -ENODATA;
 
-                r = cg_get_keyed_attribute("cpu", u->cgroup_path, "cpu.stat", keys, &val);
+                r = cg_get_keyed_attribute("cpu", u->cgroup_path, "cpu.stat", STRV_MAKE("usage_usec"), &val);
                 if (r < 0)
                         return r;
+                if (IN_SET(r, -ENOENT, -ENXIO))
+                        return -ENODATA;
 
                 r = safe_atou64(val, &us);
                 if (r < 0)