From 017f9bc1d77d5026c3c009c7246d6c743197a41e Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 27 Mar 2025 03:56:46 +0900 Subject: [PATCH 1/3] test-watch-pid: move required test conditions to intro() Follow-up for 7bcd76513a61f8f16f2cc9b291895779c7c367fe. --- src/test/test-watch-pid.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/test/test-watch-pid.c b/src/test/test-watch-pid.c index 17dce05678..2769377c7f 100644 --- a/src/test/test-watch-pid.c +++ b/src/test/test-watch-pid.c @@ -7,17 +7,10 @@ #include "service.h" #include "tests.h" -TEST_RET(watch_pid) { +TEST(watch_pid) { _cleanup_(rm_rf_physical_and_freep) char *runtime_dir = NULL; _cleanup_(manager_freep) Manager *m = NULL; Unit *a, *b, *c, *u; - int r; - - if (getuid() != 0) - return log_tests_skipped("not root"); - r = enter_cgroup_subroot(NULL); - if (r < 0) - return log_tests_skipped("cgroupfs not available"); _cleanup_free_ char *unit_dir = NULL; ASSERT_OK(get_testdata_dir("units/", &unit_dir)); @@ -95,12 +88,18 @@ TEST_RET(watch_pid) { unit_unwatch_pid(c, pid); ASSERT_NULL(manager_get_unit_by_pid(m, pid)); - - return EXIT_SUCCESS; } static int intro(void) { - log_show_color(true); + int r; + + if (getuid() != 0) + return log_tests_skipped("not root"); + + r = enter_cgroup_subroot(NULL); + if (r < 0) + return log_tests_skipped_errno(r, "cgroupfs not available"); + return EXIT_SUCCESS; } From 56691ea7537adddd163924392764ebdf4970e42f Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 27 Mar 2025 04:02:26 +0900 Subject: [PATCH 2/3] test-watch-pid: use pidref_safe_fork() with FORK_FREEZE Then, use pidref version of functions. --- src/test/test-watch-pid.c | 61 +++++++++++++++++---------------------- 1 file changed, 27 insertions(+), 34 deletions(-) diff --git a/src/test/test-watch-pid.c b/src/test/test-watch-pid.c index 2769377c7f..dd8ad11fa4 100644 --- a/src/test/test-watch-pid.c +++ b/src/test/test-watch-pid.c @@ -34,60 +34,53 @@ TEST(watch_pid) { ASSERT_TRUE(set_isempty(c->pids)); /* Fork off a child so that we have a PID to watch */ - _cleanup_(sigkill_waitp) pid_t pid = 0; - pid = fork(); - if (pid == 0) { - /* Child */ - pause(); - _exit(EXIT_SUCCESS); - } - - ASSERT_OK(pid); + _cleanup_(pidref_done_sigkill_wait) PidRef pidref = PIDREF_NULL; + ASSERT_OK_POSITIVE(pidref_safe_fork("(child)", FORK_FREEZE, &pidref)); ASSERT_TRUE(hashmap_isempty(m->watch_pids)); - ASSERT_NULL(manager_get_unit_by_pid(m, pid)); + ASSERT_NULL(manager_get_unit_by_pidref(m, &pidref)); - ASSERT_OK(unit_watch_pid(a, pid, false)); - ASSERT_PTR_EQ(manager_get_unit_by_pid(m, pid), a); + ASSERT_OK(unit_watch_pidref(a, &pidref, false)); + ASSERT_PTR_EQ(manager_get_unit_by_pidref(m, &pidref), a); - ASSERT_OK(unit_watch_pid(a, pid, false)); - ASSERT_PTR_EQ(manager_get_unit_by_pid(m, pid), a); + ASSERT_OK(unit_watch_pidref(a, &pidref, false)); + ASSERT_PTR_EQ(manager_get_unit_by_pidref(m, &pidref), a); - ASSERT_OK(unit_watch_pid(b, pid, false)); - u = manager_get_unit_by_pid(m, pid); + ASSERT_OK(unit_watch_pidref(b, &pidref, false)); + u = manager_get_unit_by_pidref(m, &pidref); ASSERT_TRUE(u == a || u == b); - ASSERT_OK(unit_watch_pid(b, pid, false)); - u = manager_get_unit_by_pid(m, pid); + ASSERT_OK(unit_watch_pidref(b, &pidref, false)); + u = manager_get_unit_by_pidref(m, &pidref); ASSERT_TRUE(u == a || u == b); - ASSERT_OK(unit_watch_pid(c, pid, false)); - u = manager_get_unit_by_pid(m, pid); + ASSERT_OK(unit_watch_pidref(c, &pidref, false)); + u = manager_get_unit_by_pidref(m, &pidref); ASSERT_TRUE(u == a || u == b || u == c); - ASSERT_OK(unit_watch_pid(c, pid, false)); - u = manager_get_unit_by_pid(m, pid); + ASSERT_OK(unit_watch_pidref(c, &pidref, false)); + u = manager_get_unit_by_pidref(m, &pidref); ASSERT_TRUE(u == a || u == b || u == c); - unit_unwatch_pid(b, pid); - u = manager_get_unit_by_pid(m, pid); + unit_unwatch_pidref(b, &pidref); + u = manager_get_unit_by_pidref(m, &pidref); ASSERT_TRUE(u == a || u == c); - unit_unwatch_pid(b, pid); - u = manager_get_unit_by_pid(m, pid); + unit_unwatch_pidref(b, &pidref); + u = manager_get_unit_by_pidref(m, &pidref); ASSERT_TRUE(u == a || u == c); - unit_unwatch_pid(a, pid); - ASSERT_PTR_EQ(manager_get_unit_by_pid(m, pid), c); + unit_unwatch_pidref(a, &pidref); + ASSERT_PTR_EQ(manager_get_unit_by_pidref(m, &pidref), c); - unit_unwatch_pid(a, pid); - ASSERT_PTR_EQ(manager_get_unit_by_pid(m, pid), c); + unit_unwatch_pidref(a, &pidref); + ASSERT_PTR_EQ(manager_get_unit_by_pidref(m, &pidref), c); - unit_unwatch_pid(c, pid); - ASSERT_NULL(manager_get_unit_by_pid(m, pid)); + unit_unwatch_pidref(c, &pidref); + ASSERT_NULL(manager_get_unit_by_pidref(m, &pidref)); - unit_unwatch_pid(c, pid); - ASSERT_NULL(manager_get_unit_by_pid(m, pid)); + unit_unwatch_pidref(c, &pidref); + ASSERT_NULL(manager_get_unit_by_pidref(m, &pidref)); } static int intro(void) { From eb3554666eacd8fbbd9f21b548026a06068137cf Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 27 Mar 2025 04:15:43 +0900 Subject: [PATCH 3/3] core: drop unused wrappers of manager_get_unit_by_pidref() and friends --- src/core/cgroup.c | 11 +---------- src/core/cgroup.h | 7 +++---- src/core/manager.c | 2 +- src/core/unit.c | 18 ------------------ src/core/unit.h | 2 -- 5 files changed, 5 insertions(+), 35 deletions(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 0bc7a17433..cb90a182f4 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -4471,7 +4471,7 @@ Unit *manager_get_unit_by_pidref_cgroup(Manager *m, const PidRef *pid) { return manager_get_unit_by_cgroup(m, cgroup); } -Unit *manager_get_unit_by_pidref_watching(Manager *m, const PidRef *pid) { +Unit* manager_get_unit_by_pidref_watching(Manager *m, const PidRef *pid) { Unit *u, **array; assert(m); @@ -4519,15 +4519,6 @@ Unit* manager_get_unit_by_pidref(Manager *m, PidRef *pid) { return NULL; } -Unit *manager_get_unit_by_pid(Manager *m, pid_t pid) { - assert(m); - - if (!pid_is_valid(pid)) - return NULL; - - return manager_get_unit_by_pidref(m, &PIDREF_MAKE_FROM_PID(pid)); -} - int manager_notify_cgroup_empty(Manager *m, const char *cgroup) { Unit *u; diff --git a/src/core/cgroup.h b/src/core/cgroup.h index 0828916319..b25cdad7d8 100644 --- a/src/core/cgroup.h +++ b/src/core/cgroup.h @@ -463,11 +463,10 @@ void manager_shutdown_cgroup(Manager *m, bool delete); unsigned manager_dispatch_cgroup_realize_queue(Manager *m); -Unit *manager_get_unit_by_cgroup(Manager *m, const char *cgroup); -Unit *manager_get_unit_by_pidref_cgroup(Manager *m, const PidRef *pid); -Unit *manager_get_unit_by_pidref_watching(Manager *m, const PidRef *pid); +Unit* manager_get_unit_by_cgroup(Manager *m, const char *cgroup); +Unit* manager_get_unit_by_pidref_cgroup(Manager *m, const PidRef *pid); +Unit* manager_get_unit_by_pidref_watching(Manager *m, const PidRef *pid); Unit* manager_get_unit_by_pidref(Manager *m, PidRef *pid); -Unit* manager_get_unit_by_pid(Manager *m, pid_t pid); uint64_t unit_get_ancestor_memory_min(Unit *u); uint64_t unit_get_ancestor_memory_low(Unit *u); diff --git a/src/core/manager.c b/src/core/manager.c index d335938b4b..110f20455d 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -2881,7 +2881,7 @@ static void manager_invoke_sigchld_event( u->sigchldgen = m->sigchldgen; log_unit_debug(u, "Child "PID_FMT" belongs to %s.", si->si_pid, u->id); - unit_unwatch_pid(u, si->si_pid); + unit_unwatch_pidref(u, &PIDREF_MAKE_FROM_PID(si->si_pid)); if (UNIT_VTABLE(u)->sigchld_event) UNIT_VTABLE(u)->sigchld_event(u, si->si_pid, si->si_code, si->si_status); diff --git a/src/core/unit.c b/src/core/unit.c index c1a3b373a1..1254091d74 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -2820,20 +2820,6 @@ int unit_watch_pidref(Unit *u, const PidRef *pid, bool exclusive) { return 0; } -int unit_watch_pid(Unit *u, pid_t pid, bool exclusive) { - _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; - int r; - - assert(u); - assert(pid_is_valid(pid)); - - r = pidref_set_pid(&pidref, pid); - if (r < 0) - return r; - - return unit_watch_pidref(u, &pidref, exclusive); -} - void unit_unwatch_pidref(Unit *u, const PidRef *pid) { assert(u); assert(pidref_is_set(pid)); @@ -2883,10 +2869,6 @@ void unit_unwatch_pidref(Unit *u, const PidRef *pid) { } } -void unit_unwatch_pid(Unit *u, pid_t pid) { - return unit_unwatch_pidref(u, &PIDREF_MAKE_FROM_PID(pid)); -} - void unit_unwatch_all_pids(Unit *u) { assert(u); diff --git a/src/core/unit.h b/src/core/unit.h index dbaeebda8e..822c39e5d1 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -876,9 +876,7 @@ void unit_notify_cgroup_oom(Unit *u, bool managed_oom); void unit_notify(Unit *u, UnitActiveState os, UnitActiveState ns, bool reload_success); int unit_watch_pidref(Unit *u, const PidRef *pid, bool exclusive); -int unit_watch_pid(Unit *u, pid_t pid, bool exclusive); void unit_unwatch_pidref(Unit *u, const PidRef *pid); -void unit_unwatch_pid(Unit *u, pid_t pid); void unit_unwatch_all_pids(Unit *u); void unit_unwatch_pidref_done(Unit *u, PidRef *pidref);