From 7b4022bdb008b9307e1664b7cf7f61f5675d2e8e Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Tue, 27 May 2025 14:15:11 +0200 Subject: [PATCH 1/3] cgroup-util: rename cg_get_xattr_malloc -> cg_get_xattr This is not a trivial wrapper around user-created-buffer-based syscall, so do not use _malloc suffix. Most of our functions return an internally allocated buffer and this one's not special. --- src/basic/cgroup-util.c | 2 +- src/basic/cgroup-util.h | 2 +- src/core/cgroup.c | 4 ++-- src/journal/journald-client.c | 2 +- src/oom/oomd-util.c | 2 +- src/oom/test-oomd-util.c | 4 ++-- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index 4ceec690ab..0c39660a9a 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -592,7 +592,7 @@ int cg_set_xattr(const char *path, const char *name, const void *value, size_t s return RET_NERRNO(setxattr(fs, name, value, size, flags)); } -int cg_get_xattr_malloc(const char *path, const char *name, char **ret, size_t *ret_size) { +int cg_get_xattr(const char *path, const char *name, char **ret, size_t *ret_size) { _cleanup_free_ char *fs = NULL; int r; diff --git a/src/basic/cgroup-util.h b/src/basic/cgroup-util.h index 27778dabed..6a5f92a177 100644 --- a/src/basic/cgroup-util.h +++ b/src/basic/cgroup-util.h @@ -226,7 +226,7 @@ int cg_get_attribute_as_bool(const char *controller, const char *path, const cha int cg_get_owner(const char *path, uid_t *ret_uid); int cg_set_xattr(const char *path, const char *name, const void *value, size_t size, int flags); -int cg_get_xattr_malloc(const char *path, const char *name, char **ret, size_t *ret_size); +int cg_get_xattr(const char *path, const char *name, char **ret, size_t *ret_size); /* Returns negative on error, and 0 or 1 on success for the bool value */ int cg_get_xattr_bool(const char *path, const char *name); int cg_remove_xattr(const char *path, const char *name); diff --git a/src/core/cgroup.c b/src/core/cgroup.c index abaffb22cd..d7d5983c8d 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -3020,7 +3020,7 @@ int unit_check_oomd_kill(Unit *u) { if (!crt || !crt->cgroup_path) return 0; - r = cg_get_xattr_malloc(crt->cgroup_path, "user.oomd_ooms", &value, /* ret_size= */ NULL); + r = cg_get_xattr(crt->cgroup_path, "user.oomd_ooms", &value, /* ret_size= */ NULL); if (r < 0 && !ERRNO_IS_XATTR_ABSENT(r)) return r; @@ -3038,7 +3038,7 @@ int unit_check_oomd_kill(Unit *u) { n = 0; value = mfree(value); - r = cg_get_xattr_malloc(crt->cgroup_path, "user.oomd_kill", &value, /* ret_size= */ NULL); + r = cg_get_xattr(crt->cgroup_path, "user.oomd_kill", &value, /* ret_size= */ NULL); if (r >= 0 && !isempty(value)) (void) safe_atou64(value, &n); diff --git a/src/journal/journald-client.c b/src/journal/journald-client.c index 1611395e94..659b9c5e88 100644 --- a/src/journal/journald-client.c +++ b/src/journal/journald-client.c @@ -61,7 +61,7 @@ int client_context_read_log_filter_patterns(ClientContext *c, const char *cgroup _cleanup_free_ char *xattr = NULL; size_t xattr_size = 0; - r = cg_get_xattr_malloc(unit_cgroup, "user.journald_log_filter_patterns", &xattr, &xattr_size); + r = cg_get_xattr(unit_cgroup, "user.journald_log_filter_patterns", &xattr, &xattr_size); if (ERRNO_IS_NEG_XATTR_ABSENT(r)) { client_set_filtering_patterns(c, /* allow_list= */ NULL, /* deny_list= */ NULL); return 0; diff --git a/src/oom/oomd-util.c b/src/oom/oomd-util.c index de0f2e6854..4b0e4a54aa 100644 --- a/src/oom/oomd-util.c +++ b/src/oom/oomd-util.c @@ -41,7 +41,7 @@ static int increment_oomd_xattr(const char *path, const char *xattr, uint64_t nu assert(path); assert(xattr); - r = cg_get_xattr_malloc(path, xattr, &value, /* ret_size= */ NULL); + r = cg_get_xattr(path, xattr, &value, /* ret_size= */ NULL); if (r < 0 && !ERRNO_IS_XATTR_ABSENT(r)) return r; diff --git a/src/oom/test-oomd-util.c b/src/oom/test-oomd-util.c index 3c5159129f..6ab6f101ff 100644 --- a/src/oom/test-oomd-util.c +++ b/src/oom/test-oomd-util.c @@ -74,7 +74,7 @@ static void test_oomd_cgroup_kill(void) { abort(); } - ASSERT_OK(cg_get_xattr_malloc(cgroup, "user.oomd_ooms", &v, /* ret_size= */ NULL)); + ASSERT_OK(cg_get_xattr(cgroup, "user.oomd_ooms", &v, /* ret_size= */ NULL)); assert_se(streq(v, i == 0 ? "1" : "2")); v = mfree(v); @@ -82,7 +82,7 @@ static void test_oomd_cgroup_kill(void) { sleep(2); assert_se(cg_is_empty(SYSTEMD_CGROUP_CONTROLLER, cgroup) == true); - ASSERT_OK(cg_get_xattr_malloc(cgroup, "user.oomd_kill", &v, /* ret_size= */ NULL)); + ASSERT_OK(cg_get_xattr(cgroup, "user.oomd_kill", &v, /* ret_size= */ NULL)); assert_se(streq(v, i == 0 ? "2" : "4")); } } From 606ef91a407ba439fe1d05c3843878dd7e4dc291 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Tue, 27 May 2025 14:19:41 +0200 Subject: [PATCH 2/3] cgroup-util: make cg_get_attribute_as_bool() return 0/1 directly As with getenv_bool() and getxattr_at_bool(), to match our usual style. --- src/basic/cgroup-util.c | 11 ++--------- src/basic/cgroup-util.h | 4 +--- src/oom/oomd-manager.c | 6 ++---- 3 files changed, 5 insertions(+), 16 deletions(-) diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index 0c39660a9a..c70b04650c 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -1638,24 +1638,17 @@ int cg_get_attribute_as_uint64(const char *controller, const char *path, const c return 0; } -int cg_get_attribute_as_bool(const char *controller, const char *path, const char *attribute, bool *ret) { +int cg_get_attribute_as_bool(const char *controller, const char *path, const char *attribute) { _cleanup_free_ char *value = NULL; int r; - assert(ret); - r = cg_get_attribute(controller, path, attribute, &value); if (r == -ENOENT) return -ENODATA; if (r < 0) return r; - r = parse_boolean(value); - if (r < 0) - return r; - - *ret = r; - return 0; + return parse_boolean(value); } int cg_get_owner(const char *path, uid_t *ret_uid) { diff --git a/src/basic/cgroup-util.h b/src/basic/cgroup-util.h index 6a5f92a177..7c5e6e6555 100644 --- a/src/basic/cgroup-util.h +++ b/src/basic/cgroup-util.h @@ -219,9 +219,7 @@ int cg_get_attribute(const char *controller, const char *path, const char *attri int cg_get_keyed_attribute(const char *controller, const char *path, const char *attribute, char * const *keys, char **values); int cg_get_attribute_as_uint64(const char *controller, const char *path, const char *attribute, uint64_t *ret); - -/* Does a parse_boolean() on the attribute contents and sets ret accordingly */ -int cg_get_attribute_as_bool(const char *controller, const char *path, const char *attribute, bool *ret); +int cg_get_attribute_as_bool(const char *controller, const char *path, const char *attribute); int cg_get_owner(const char *path, uid_t *ret_uid); diff --git a/src/oom/oomd-manager.c b/src/oom/oomd-manager.c index dae7a957e2..51fb5fe9da 100644 --- a/src/oom/oomd-manager.c +++ b/src/oom/oomd-manager.c @@ -228,7 +228,6 @@ static int recursively_get_cgroup_context(Hashmap *new_h, const char *path) { do { _cleanup_free_ char *cg_path = NULL; - bool oom_group; cg_path = path_join(empty_to_root(path), subpath); if (!cg_path) @@ -236,7 +235,7 @@ static int recursively_get_cgroup_context(Hashmap *new_h, const char *path) { subpath = mfree(subpath); - r = cg_get_attribute_as_bool("memory", cg_path, "memory.oom.group", &oom_group); + r = cg_get_attribute_as_bool("memory", cg_path, "memory.oom.group"); /* The cgroup might be gone. Skip it as a candidate since we can't get information on it. */ if (r == -ENOMEM) return r; @@ -244,8 +243,7 @@ static int recursively_get_cgroup_context(Hashmap *new_h, const char *path) { log_debug_errno(r, "Failed to read memory.oom.group from %s, ignoring: %m", cg_path); return 0; } - - if (oom_group) + if (r > 0) r = oomd_insert_cgroup_context(NULL, new_h, cg_path); else r = recursively_get_cgroup_context(new_h, cg_path); From a941822d1c00f0275f0f1b5ef96d4c533b63c8de Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Tue, 27 May 2025 14:25:12 +0200 Subject: [PATCH 3/3] cgroup-util: assert on attribute for cg_{get,set}_attribute() It makes zero sense to ever perform IO operations on cgroup dirs. --- src/basic/cgroup-util.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index c70b04650c..f3ce174244 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -1594,6 +1594,8 @@ int cg_set_attribute(const char *controller, const char *path, const char *attri _cleanup_free_ char *p = NULL; int r; + assert(attribute); + r = cg_get_path(controller, path, attribute, &p); if (r < 0) return r; @@ -1605,6 +1607,8 @@ int cg_get_attribute(const char *controller, const char *path, const char *attri _cleanup_free_ char *p = NULL; int r; + assert(attribute); + r = cg_get_path(controller, path, attribute, &p); if (r < 0) return r;