From 385eccf65b6acb9a30b41311272d45705cd0958f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 20 Nov 2024 09:38:52 +0100 Subject: [PATCH 1/2] logind: drop one duplicate param in manager_is_inhibited() In the review in https://github.com/systemd/systemd/pull/30307#pullrequestreview-2255002732 removal of the excessive boolean parameters was requested. We don't need a separate boolean param here, since we always pass true with a uid and false otherwise. --- src/login/logind-action.c | 4 ++-- src/login/logind-core.c | 2 +- src/login/logind-dbus.c | 8 ++++---- src/login/logind-inhibit.c | 10 +++++----- src/login/logind-inhibit.h | 9 ++++++++- 5 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/login/logind-action.c b/src/login/logind-action.c index db9a0e1bbae..8b3c345e7ab 100644 --- a/src/login/logind-action.c +++ b/src/login/logind-action.c @@ -234,7 +234,7 @@ static int handle_action_execute( /* If the actual operation is inhibited, warn and fail */ if (inhibit_what_is_valid(inhibit_operation) && !ignore_inhibited && - manager_is_inhibited(m, inhibit_operation, /* block= */ true, NULL, false, false, 0, &offending)) { + manager_is_inhibited(m, inhibit_operation, /* block= */ true, NULL, false, UID_INVALID, &offending)) { _cleanup_free_ char *comm = NULL, *u = NULL; (void) pidref_get_comm(&offending->pid, &comm); @@ -372,7 +372,7 @@ int manager_handle_action( /* If the key handling is inhibited, don't do anything */ if (inhibit_key > 0) { - if (manager_is_inhibited(m, inhibit_key, /* block= */ true, NULL, true, false, 0, NULL)) { + if (manager_is_inhibited(m, inhibit_key, /* block= */ true, NULL, true, UID_INVALID, NULL)) { log_debug("Refusing %s operation, %s is inhibited.", handle_action_to_string(handle), inhibit_what_to_string(inhibit_key)); diff --git a/src/login/logind-core.c b/src/login/logind-core.c index fad276f195e..e86f1142446 100644 --- a/src/login/logind-core.c +++ b/src/login/logind-core.c @@ -411,7 +411,7 @@ int manager_get_idle_hint(Manager *m, dual_timestamp *t) { assert(m); - idle_hint = !manager_is_inhibited(m, INHIBIT_IDLE, /* block= */ true, t, false, false, 0, NULL); + idle_hint = !manager_is_inhibited(m, INHIBIT_IDLE, /* block= */ true, t, false, UID_INVALID, NULL); HASHMAP_FOREACH(s, m->sessions) { dual_timestamp k; diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index 80a2470a71f..e14b681e9b4 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -1931,7 +1931,7 @@ int manager_dispatch_delayed(Manager *manager, bool timeout) { if (!manager->delayed_action || manager->action_job) return 0; - if (manager_is_inhibited(manager, manager->delayed_action->inhibit_what, /* block= */ false, NULL, false, false, 0, &offending)) { + if (manager_is_inhibited(manager, manager->delayed_action->inhibit_what, /* block= */ false, NULL, false, UID_INVALID, &offending)) { _cleanup_free_ char *comm = NULL, *u = NULL; if (!timeout) @@ -2033,7 +2033,7 @@ int bus_manager_shutdown_or_sleep_now_or_later( delayed = m->inhibit_delay_max > 0 && - manager_is_inhibited(m, a->inhibit_what, /* block= */ false, NULL, false, false, 0, NULL); + manager_is_inhibited(m, a->inhibit_what, /* block= */ false, NULL, false, UID_INVALID, NULL); if (delayed) /* Shutdown is delayed, keep in mind what we @@ -2077,7 +2077,7 @@ static int verify_shutdown_creds( return r; multiple_sessions = r > 0; - blocked = manager_is_inhibited(m, a->inhibit_what, /* block= */ true, NULL, false, true, uid, &offending); + blocked = manager_is_inhibited(m, a->inhibit_what, /* block= */ true, NULL, false, uid, &offending); interactive = flags & SD_LOGIND_INTERACTIVE; if (multiple_sessions) { @@ -2820,7 +2820,7 @@ static int method_can_shutdown_or_sleep( return r; multiple_sessions = r > 0; - blocked = manager_is_inhibited(m, a->inhibit_what, /* block= */ true, NULL, false, true, uid, NULL); + blocked = manager_is_inhibited(m, a->inhibit_what, /* block= */ true, NULL, false, uid, NULL); if (check_unit_state && a->target) { _cleanup_free_ char *load_state = NULL; diff --git a/src/login/logind-inhibit.c b/src/login/logind-inhibit.c index 887bfb7eaf0..e4d2e7a73f8 100644 --- a/src/login/logind-inhibit.c +++ b/src/login/logind-inhibit.c @@ -402,8 +402,7 @@ bool manager_is_inhibited( bool block, dual_timestamp *since, bool ignore_inactive, - bool ignore_uid, - uid_t uid, + uid_t uid_to_ignore, Inhibitor **ret_offending) { Inhibitor *i, *offending = NULL; @@ -428,11 +427,12 @@ bool manager_is_inhibited( if (ignore_inactive && pidref_is_active_session(m, &i->pid) <= 0) continue; - if (i->mode == INHIBIT_BLOCK_WEAK && ignore_uid && i->uid == uid) + if (i->mode == INHIBIT_BLOCK_WEAK && + uid_is_valid(uid_to_ignore) && + uid_to_ignore == i->uid) continue; - if (!inhibited || - i->since.monotonic < ts.monotonic) + if (!inhibited || i->since.monotonic < ts.monotonic) ts = i->since; inhibited = true; diff --git a/src/login/logind-inhibit.h b/src/login/logind-inhibit.h index 16abd6958cc..6b4d7a0689d 100644 --- a/src/login/logind-inhibit.h +++ b/src/login/logind-inhibit.h @@ -67,7 +67,14 @@ int inhibitor_create_fifo(Inhibitor *i); bool inhibitor_is_orphan(Inhibitor *i); InhibitWhat manager_inhibit_what(Manager *m, InhibitMode mode); -bool manager_is_inhibited(Manager *m, InhibitWhat w, bool block, dual_timestamp *since, bool ignore_inactive, bool ignore_uid, uid_t uid, Inhibitor **offending); +bool manager_is_inhibited( + Manager *m, + InhibitWhat w, + bool block, + dual_timestamp *since, + bool ignore_inactive, + uid_t uid_to_ignore, + Inhibitor **ret_offending); static inline bool inhibit_what_is_valid(InhibitWhat w) { return w > 0 && w < _INHIBIT_WHAT_MAX; From 0c1622aa5afc1d44ca9569000fb57240518031ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 20 Nov 2024 13:15:06 +0100 Subject: [PATCH 2/2] logind: define flags enum for manager_is_inhibited() The most common case of block=true, ignore_inactive=false is mapped to flags=0. For https://github.com/systemd/systemd/issues/34091. --- src/login/logind-action.c | 4 ++-- src/login/logind-core.c | 2 +- src/login/logind-dbus.c | 13 +++++++++---- src/login/logind-inhibit.c | 9 ++++----- src/login/logind-inhibit.h | 10 ++++++++-- 5 files changed, 24 insertions(+), 14 deletions(-) diff --git a/src/login/logind-action.c b/src/login/logind-action.c index 8b3c345e7ab..712438f4b1e 100644 --- a/src/login/logind-action.c +++ b/src/login/logind-action.c @@ -234,7 +234,7 @@ static int handle_action_execute( /* If the actual operation is inhibited, warn and fail */ if (inhibit_what_is_valid(inhibit_operation) && !ignore_inhibited && - manager_is_inhibited(m, inhibit_operation, /* block= */ true, NULL, false, UID_INVALID, &offending)) { + manager_is_inhibited(m, inhibit_operation, NULL, /* flags= */ 0, UID_INVALID, &offending)) { _cleanup_free_ char *comm = NULL, *u = NULL; (void) pidref_get_comm(&offending->pid, &comm); @@ -372,7 +372,7 @@ int manager_handle_action( /* If the key handling is inhibited, don't do anything */ if (inhibit_key > 0) { - if (manager_is_inhibited(m, inhibit_key, /* block= */ true, NULL, true, UID_INVALID, NULL)) { + if (manager_is_inhibited(m, inhibit_key, NULL, MANAGER_IS_INHIBITED_IGNORE_INACTIVE, UID_INVALID, NULL)) { log_debug("Refusing %s operation, %s is inhibited.", handle_action_to_string(handle), inhibit_what_to_string(inhibit_key)); diff --git a/src/login/logind-core.c b/src/login/logind-core.c index e86f1142446..bcb82582e64 100644 --- a/src/login/logind-core.c +++ b/src/login/logind-core.c @@ -411,7 +411,7 @@ int manager_get_idle_hint(Manager *m, dual_timestamp *t) { assert(m); - idle_hint = !manager_is_inhibited(m, INHIBIT_IDLE, /* block= */ true, t, false, UID_INVALID, NULL); + idle_hint = !manager_is_inhibited(m, INHIBIT_IDLE, t, /* flags= */ 0, UID_INVALID, NULL); HASHMAP_FOREACH(s, m->sessions) { dual_timestamp k; diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index e14b681e9b4..78b4783faa9 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -1931,7 +1931,12 @@ int manager_dispatch_delayed(Manager *manager, bool timeout) { if (!manager->delayed_action || manager->action_job) return 0; - if (manager_is_inhibited(manager, manager->delayed_action->inhibit_what, /* block= */ false, NULL, false, UID_INVALID, &offending)) { + if (manager_is_inhibited(manager, + manager->delayed_action->inhibit_what, + NULL, + MANAGER_IS_INHIBITED_CHECK_DELAY, + UID_INVALID, + &offending)) { _cleanup_free_ char *comm = NULL, *u = NULL; if (!timeout) @@ -2033,7 +2038,7 @@ int bus_manager_shutdown_or_sleep_now_or_later( delayed = m->inhibit_delay_max > 0 && - manager_is_inhibited(m, a->inhibit_what, /* block= */ false, NULL, false, UID_INVALID, NULL); + manager_is_inhibited(m, a->inhibit_what, NULL, MANAGER_IS_INHIBITED_CHECK_DELAY, UID_INVALID, NULL); if (delayed) /* Shutdown is delayed, keep in mind what we @@ -2077,7 +2082,7 @@ static int verify_shutdown_creds( return r; multiple_sessions = r > 0; - blocked = manager_is_inhibited(m, a->inhibit_what, /* block= */ true, NULL, false, uid, &offending); + blocked = manager_is_inhibited(m, a->inhibit_what, NULL, /* flags= */ 0, uid, &offending); interactive = flags & SD_LOGIND_INTERACTIVE; if (multiple_sessions) { @@ -2820,7 +2825,7 @@ static int method_can_shutdown_or_sleep( return r; multiple_sessions = r > 0; - blocked = manager_is_inhibited(m, a->inhibit_what, /* block= */ true, NULL, false, uid, NULL); + blocked = manager_is_inhibited(m, a->inhibit_what, NULL, /* flags= */ 0, uid, NULL); if (check_unit_state && a->target) { _cleanup_free_ char *load_state = NULL; diff --git a/src/login/logind-inhibit.c b/src/login/logind-inhibit.c index e4d2e7a73f8..ec870d10016 100644 --- a/src/login/logind-inhibit.c +++ b/src/login/logind-inhibit.c @@ -399,9 +399,8 @@ static int pidref_is_active_session(Manager *m, const PidRef *pid) { bool manager_is_inhibited( Manager *m, InhibitWhat w, - bool block, dual_timestamp *since, - bool ignore_inactive, + ManagerIsInhibitedFlags flags, uid_t uid_to_ignore, Inhibitor **ret_offending) { @@ -420,11 +419,11 @@ bool manager_is_inhibited( if (!(i->what & w)) continue; - if ((block && !IN_SET(i->mode, INHIBIT_BLOCK, INHIBIT_BLOCK_WEAK)) || - (!block && i->mode != INHIBIT_DELAY)) + if ((flags & MANAGER_IS_INHIBITED_CHECK_DELAY) != (i->mode == INHIBIT_DELAY)) continue; - if (ignore_inactive && pidref_is_active_session(m, &i->pid) <= 0) + if ((flags & MANAGER_IS_INHIBITED_IGNORE_INACTIVE) && + pidref_is_active_session(m, &i->pid) <= 0) continue; if (i->mode == INHIBIT_BLOCK_WEAK && diff --git a/src/login/logind-inhibit.h b/src/login/logind-inhibit.h index 6b4d7a0689d..ae91e2d402e 100644 --- a/src/login/logind-inhibit.h +++ b/src/login/logind-inhibit.h @@ -67,12 +67,18 @@ int inhibitor_create_fifo(Inhibitor *i); bool inhibitor_is_orphan(Inhibitor *i); InhibitWhat manager_inhibit_what(Manager *m, InhibitMode mode); + +typedef enum ManagerIsInhibitedFlags { + MANAGER_IS_INHIBITED_CHECK_DELAY = 1 << 0, /* When set, we only check delay inhibitors. + * Otherwise, we only check block inhibitors. */ + MANAGER_IS_INHIBITED_IGNORE_INACTIVE = 1 << 1, /* When set, ignore inactive sessions. */ +} ManagerIsInhibitedFlags; + bool manager_is_inhibited( Manager *m, InhibitWhat w, - bool block, dual_timestamp *since, - bool ignore_inactive, + ManagerIsInhibitedFlags flags, uid_t uid_to_ignore, Inhibitor **ret_offending);