From 3bbb76f62123f59ad871bebd00fdcd3f0c974275 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 8 Apr 2021 14:59:10 +0200 Subject: [PATCH 1/2] sd-bus: make sd_bus_is_{ready,open} accept NULL We didn't document this behaviour one way or another, so I think it's OK to change. All callers do the NULL check before callling this to avoid the assert warning, so it seems reasonable to do it internally. sd_bus_can_send() is similar, but there we expressly say that an error is returned on NULL, so I didn't change it. --- man/sd_bus_is_open.xml | 7 +++++-- src/libsystemd/sd-bus/sd-bus.c | 8 ++++++-- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/man/sd_bus_is_open.xml b/man/sd_bus_is_open.xml index 8e0aed29b3..621ed272bb 100644 --- a/man/sd_bus_is_open.xml +++ b/man/sd_bus_is_open.xml @@ -57,6 +57,9 @@ zero outside of this state, and positive otherwise. Effectively, this function returns positive while regular messages can be sent or received on the connection. + The bus argument may be NULL, zero is also returned in + that case. + To be notified when the connection is fully established, use sd_bus_set_connected_signal3 and install a match for the Connected() signal on the @@ -68,8 +71,8 @@ Return Value - On success, these functions return 0 or a positive integer. On failure, they return a negative errno-style - error code. + Those functions return 0 if the bus is not in the given state, and a positive + integer when it is. On failure, a negative errno-style error code is returned. Errors diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index e719c74370..2f4e67c97e 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -1800,7 +1800,9 @@ void bus_enter_closing(sd_bus *bus) { DEFINE_PUBLIC_TRIVIAL_REF_UNREF_FUNC(sd_bus, sd_bus, bus_free); _public_ int sd_bus_is_open(sd_bus *bus) { - assert_return(bus, -EINVAL); + if (!bus) + return 0; + assert_return(bus = bus_resolve(bus), -ENOPKG); assert_return(!bus_pid_changed(bus), -ECHILD); @@ -1808,7 +1810,9 @@ _public_ int sd_bus_is_open(sd_bus *bus) { } _public_ int sd_bus_is_ready(sd_bus *bus) { - assert_return(bus, -EINVAL); + if (!bus) + return 0; + assert_return(bus = bus_resolve(bus), -ENOPKG); assert_return(!bus_pid_changed(bus), -ECHILD); From 5dbec9bd3223c908949c20a97d6d280c231995b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 8 Apr 2021 15:04:18 +0200 Subject: [PATCH 2/2] networkd: simplify sd_bus_is_ready() checks Also add "system" in the messages, because we set the internal value, and are just skipping the setting of the external value, so the message could be confusing without that clarification. --- src/network/networkd-dhcp-common.c | 2 +- src/network/networkd-link-bus.c | 2 +- src/network/networkd-manager-bus.c | 2 +- src/network/networkd-manager.c | 9 ++++----- 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/network/networkd-dhcp-common.c b/src/network/networkd-dhcp-common.c index 0de1892fc9..9677dfb0c3 100644 --- a/src/network/networkd-dhcp-common.c +++ b/src/network/networkd-dhcp-common.c @@ -167,7 +167,7 @@ int manager_request_product_uuid(Manager *m, Link *link) { return log_oom(); } - if (!m->bus || sd_bus_is_ready(m->bus) <= 0) { + if (sd_bus_is_ready(m->bus) <= 0) { log_debug("Not connected to system bus, requesting product UUID later."); return 0; } diff --git a/src/network/networkd-link-bus.c b/src/network/networkd-link-bus.c index 841d76b461..3311dafd6a 100644 --- a/src/network/networkd-link-bus.c +++ b/src/network/networkd-link-bus.c @@ -854,7 +854,7 @@ int link_send_changed_strv(Link *link, char **properties) { assert(link->manager); assert(properties); - if (!link->manager->bus || sd_bus_is_ready(link->manager->bus) <= 0) + if (sd_bus_is_ready(link->manager->bus) <= 0) return 0; p = link_bus_path(link); diff --git a/src/network/networkd-manager-bus.c b/src/network/networkd-manager-bus.c index a08dff58e7..138d765414 100644 --- a/src/network/networkd-manager-bus.c +++ b/src/network/networkd-manager-bus.c @@ -339,7 +339,7 @@ int manager_send_changed_strv(Manager *manager, char **properties) { assert(manager); assert(properties); - if (!manager->bus || sd_bus_is_ready(manager->bus) <= 0) + if (sd_bus_is_ready(manager->bus) <= 0) return 0; return sd_bus_emit_properties_changed_strv( diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c index bfdb1f8c96..a8db2cc44b 100644 --- a/src/network/networkd-manager.c +++ b/src/network/networkd-manager.c @@ -767,8 +767,8 @@ int manager_set_hostname(Manager *m, const char *hostname) { if (r < 0) return r; - if (!m->bus || sd_bus_is_ready(m->bus) <= 0) { - log_debug("Not connected to system bus, setting hostname later."); + if (sd_bus_is_ready(m->bus) <= 0) { + log_debug("Not connected to system bus, setting system hostname later."); return 0; } @@ -784,7 +784,6 @@ int manager_set_hostname(Manager *m, const char *hostname) { "sb", hostname, false); - if (r < 0) return log_error_errno(r, "Could not set transient hostname: %m"); @@ -817,8 +816,8 @@ int manager_set_timezone(Manager *m, const char *tz) { if (r < 0) return r; - if (!m->bus || sd_bus_is_ready(m->bus) <= 0) { - log_debug("Not connected to system bus, setting timezone later."); + if (sd_bus_is_ready(m->bus) <= 0) { + log_debug("Not connected to system bus, setting system timezone later."); return 0; }