From a3d59cd1b0a2738d06893948492113f2c35be0af Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 19 Mar 2014 21:41:21 +0100 Subject: [PATCH] sd-bus: don't use assert_return() to check for disconnected bus connections A terminated connection is a runtime error and not a developer mistake, hence don't use assert_return() to check for it. --- src/libsystemd/sd-bus/bus-control.c | 20 +++++++++++++----- src/libsystemd/sd-bus/bus-convenience.c | 58 +++++++++++++++++++++++++++++++++++++++++------------- src/libsystemd/sd-bus/bus-objects.c | 23 +++++++++++++++------ src/libsystemd/sd-bus/sd-bus.c | 49 +++++++++++++++++++++++++++++++++------------ 4 files changed, 113 insertions(+), 37 deletions(-) --- src/libsystemd/sd-bus/bus-control.c +++ src/libsystemd/sd-bus/bus-control.c 2014-03-28 00:00:00.000000000 +0000 @@ -128,12 +128,14 @@ _public_ int sd_bus_request_name(sd_bus assert_return(bus, -EINVAL); assert_return(name, -EINVAL); assert_return(bus->bus_client, -EINVAL); - assert_return(BUS_IS_OPEN(bus->state), -ENOTCONN); assert_return(!bus_pid_changed(bus), -ECHILD); assert_return(!(flags & ~(SD_BUS_NAME_ALLOW_REPLACEMENT|SD_BUS_NAME_REPLACE_EXISTING|SD_BUS_NAME_QUEUE)), -EINVAL); assert_return(service_name_is_valid(name), -EINVAL); assert_return(name[0] != ':', -EINVAL); + if (!BUS_IS_OPEN(bus->state)) + return -ENOTCONN; + if (bus->is_kernel) return bus_request_name_kernel(bus, name, flags); else @@ -201,11 +203,13 @@ _public_ int sd_bus_release_name(sd_bus assert_return(bus, -EINVAL); assert_return(name, -EINVAL); assert_return(bus->bus_client, -EINVAL); - assert_return(BUS_IS_OPEN(bus->state), -ENOTCONN); assert_return(!bus_pid_changed(bus), -ECHILD); assert_return(service_name_is_valid(name), -EINVAL); assert_return(name[0] != ':', -EINVAL); + if (!BUS_IS_OPEN(bus->state)) + return -ENOTCONN; + if (bus->is_kernel) return bus_release_name_kernel(bus, name); else @@ -344,9 +348,11 @@ static int bus_list_names_dbus1(sd_bus * _public_ int sd_bus_list_names(sd_bus *bus, char ***acquired, char ***activatable) { assert_return(bus, -EINVAL); assert_return(acquired || activatable, -EINVAL); - assert_return(BUS_IS_OPEN(bus->state), -ENOTCONN); assert_return(!bus_pid_changed(bus), -ECHILD); + if (!BUS_IS_OPEN(bus->state)) + return -ENOTCONN; + if (bus->is_kernel) return bus_list_names_kernel(bus, acquired, activatable); else @@ -737,11 +743,13 @@ _public_ int sd_bus_get_owner( assert_return(name, -EINVAL); assert_return(mask <= _SD_BUS_CREDS_ALL, -ENOTSUP); assert_return(mask == 0 || creds, -EINVAL); - assert_return(BUS_IS_OPEN(bus->state), -ENOTCONN); assert_return(!bus_pid_changed(bus), -ECHILD); assert_return(service_name_is_valid(name), -EINVAL); assert_return(bus->bus_client, -ENODATA); + if (!BUS_IS_OPEN(bus->state)) + return -ENOTCONN; + if (bus->is_kernel) return bus_get_owner_kdbus(bus, name, mask, creds); else @@ -1198,10 +1206,12 @@ _public_ int sd_bus_get_owner_machine_id assert_return(bus, -EINVAL); assert_return(name, -EINVAL); assert_return(machine, -EINVAL); - assert_return(BUS_IS_OPEN(bus->state), -ENOTCONN); assert_return(!bus_pid_changed(bus), -ECHILD); assert_return(service_name_is_valid(name), -EINVAL); + if (!BUS_IS_OPEN(bus->state)) + return -ENOTCONN; + if (streq_ptr(name, bus->unique_name)) return sd_id128_get_machine(machine); --- src/libsystemd/sd-bus/bus-convenience.c +++ src/libsystemd/sd-bus/bus-convenience.c 2014-03-28 00:00:00.000000000 +0000 @@ -36,9 +36,11 @@ _public_ int sd_bus_emit_signal( int r; assert_return(bus, -EINVAL); - assert_return(BUS_IS_OPEN(bus->state), -ENOTCONN); assert_return(!bus_pid_changed(bus), -ECHILD); + if (!BUS_IS_OPEN(bus->state)) + return -ENOTCONN; + r = sd_bus_message_new_signal(bus, &m, path, interface, member); if (r < 0) return r; @@ -70,9 +72,11 @@ _public_ int sd_bus_call_method( int r; assert_return(bus, -EINVAL); - assert_return(BUS_IS_OPEN(bus->state), -ENOTCONN); assert_return(!bus_pid_changed(bus), -ECHILD); + if (!BUS_IS_OPEN(bus->state)) + return -ENOTCONN; + r = sd_bus_message_new_method_call(bus, &m, destination, path, interface, member); if (r < 0) return r; @@ -100,9 +104,12 @@ _public_ int sd_bus_reply_method_return( assert_return(call, -EINVAL); assert_return(call->sealed, -EPERM); assert_return(call->header->type == SD_BUS_MESSAGE_METHOD_CALL, -EINVAL); - assert_return(call->bus && BUS_IS_OPEN(call->bus->state), -ENOTCONN); + assert_return(call->bus, -EINVAL); assert_return(!bus_pid_changed(call->bus), -ECHILD); + if (!BUS_IS_OPEN(call->bus->state)) + return -ENOTCONN; + if (call->header->flags & BUS_MESSAGE_NO_REPLY_EXPECTED) return 0; @@ -134,9 +141,12 @@ _public_ int sd_bus_reply_method_error( assert_return(call->sealed, -EPERM); assert_return(call->header->type == SD_BUS_MESSAGE_METHOD_CALL, -EINVAL); assert_return(sd_bus_error_is_set(e), -EINVAL); - assert_return(call->bus && BUS_IS_OPEN(call->bus->state), -ENOTCONN); + assert_return(call->bus, -EINVAL); assert_return(!bus_pid_changed(call->bus), -ECHILD); + if (!BUS_IS_OPEN(call->bus->state)) + return -ENOTCONN; + if (call->header->flags & BUS_MESSAGE_NO_REPLY_EXPECTED) return 0; @@ -159,9 +169,12 @@ _public_ int sd_bus_reply_method_errorf( assert_return(call, -EINVAL); assert_return(call->sealed, -EPERM); assert_return(call->header->type == SD_BUS_MESSAGE_METHOD_CALL, -EINVAL); - assert_return(call->bus && BUS_IS_OPEN(call->bus->state), -ENOTCONN); + assert_return(call->bus, -EINVAL); assert_return(!bus_pid_changed(call->bus), -ECHILD); + if (!BUS_IS_OPEN(call->bus->state)) + return -ENOTCONN; + if (call->header->flags & BUS_MESSAGE_NO_REPLY_EXPECTED) return 0; @@ -182,9 +195,12 @@ _public_ int sd_bus_reply_method_errno( assert_return(call, -EINVAL); assert_return(call->sealed, -EPERM); assert_return(call->header->type == SD_BUS_MESSAGE_METHOD_CALL, -EINVAL); - assert_return(call->bus && BUS_IS_OPEN(call->bus->state), -ENOTCONN); + assert_return(call->bus, -EINVAL); assert_return(!bus_pid_changed(call->bus), -ECHILD); + if (!BUS_IS_OPEN(call->bus->state)) + return -ENOTCONN; + if (call->header->flags & BUS_MESSAGE_NO_REPLY_EXPECTED) return 0; @@ -208,9 +224,12 @@ _public_ int sd_bus_reply_method_errnof( assert_return(call, -EINVAL); assert_return(call->sealed, -EPERM); assert_return(call->header->type == SD_BUS_MESSAGE_METHOD_CALL, -EINVAL); - assert_return(call->bus && BUS_IS_OPEN(call->bus->state), -ENOTCONN); + assert_return(call->bus, -EINVAL); assert_return(!bus_pid_changed(call->bus), -ECHILD); + if (!BUS_IS_OPEN(call->bus->state)) + return -ENOTCONN; + if (call->header->flags & BUS_MESSAGE_NO_REPLY_EXPECTED) return 0; @@ -239,9 +258,11 @@ _public_ int sd_bus_get_property( assert_return(member_name_is_valid(member), -EINVAL); assert_return(reply, -EINVAL); assert_return(signature_is_single(type, false), -EINVAL); - assert_return(BUS_IS_OPEN(bus->state), -ENOTCONN); assert_return(!bus_pid_changed(bus), -ECHILD); + if (!BUS_IS_OPEN(bus->state)) + return -ENOTCONN; + r = sd_bus_call_method(bus, destination, path, "org.freedesktop.DBus.Properties", "Get", error, &rep, "ss", strempty(interface), member); if (r < 0) return r; @@ -273,9 +294,11 @@ _public_ int sd_bus_get_property_trivial assert_return(member_name_is_valid(member), -EINVAL); assert_return(bus_type_is_trivial(type), -EINVAL); assert_return(ptr, -EINVAL); - assert_return(BUS_IS_OPEN(bus->state), -ENOTCONN); assert_return(!bus_pid_changed(bus), -ECHILD); + if (!BUS_IS_OPEN(bus->state)) + return -ENOTCONN; + r = sd_bus_call_method(bus, destination, path, "org.freedesktop.DBus.Properties", "Get", error, &reply, "ss", strempty(interface), member); if (r < 0) return r; @@ -309,9 +332,11 @@ _public_ int sd_bus_get_property_string( assert_return(isempty(interface) || interface_name_is_valid(interface), -EINVAL); assert_return(member_name_is_valid(member), -EINVAL); assert_return(ret, -EINVAL); - assert_return(BUS_IS_OPEN(bus->state), -ENOTCONN); assert_return(!bus_pid_changed(bus), -ECHILD); + if (!BUS_IS_OPEN(bus->state)) + return -ENOTCONN; + r = sd_bus_call_method(bus, destination, path, "org.freedesktop.DBus.Properties", "Get", error, &reply, "ss", strempty(interface), member); if (r < 0) return r; @@ -348,9 +373,11 @@ _public_ int sd_bus_get_property_strv( assert_return(isempty(interface) || interface_name_is_valid(interface), -EINVAL); assert_return(member_name_is_valid(member), -EINVAL); assert_return(ret, -EINVAL); - assert_return(BUS_IS_OPEN(bus->state), -ENOTCONN); assert_return(!bus_pid_changed(bus), -ECHILD); + if (!BUS_IS_OPEN(bus->state)) + return -ENOTCONN; + r = sd_bus_call_method(bus, destination, path, "org.freedesktop.DBus.Properties", "Get", error, &reply, "ss", strempty(interface), member); if (r < 0) return r; @@ -383,9 +410,11 @@ _public_ int sd_bus_set_property( assert_return(isempty(interface) || interface_name_is_valid(interface), -EINVAL); assert_return(member_name_is_valid(member), -EINVAL); assert_return(signature_is_single(type, false), -EINVAL); - assert_return(BUS_IS_OPEN(bus->state), -ENOTCONN); assert_return(!bus_pid_changed(bus), -ECHILD); + if (!BUS_IS_OPEN(bus->state)) + return -ENOTCONN; + r = sd_bus_message_new_method_call(bus, &m, destination, path, "org.freedesktop.DBus.Properties", "Set"); if (r < 0) return r; @@ -416,9 +445,12 @@ _public_ int sd_bus_query_sender_creds(s assert_return(call, -EINVAL); assert_return(call->sealed, -EPERM); - assert_return(call->bus && BUS_IS_OPEN(call->bus->state), -ENOTCONN); + assert_return(call->bus, -EINVAL); assert_return(!bus_pid_changed(call->bus), -ECHILD); + if (!BUS_IS_OPEN(call->bus->state)) + return -ENOTCONN; + c = sd_bus_message_get_creds(call); /* All data we need? */ --- src/libsystemd/sd-bus/bus-objects.c +++ src/libsystemd/sd-bus/bus-objects.c 2014-03-28 00:00:00.000000000 +0000 @@ -2196,9 +2196,10 @@ _public_ int sd_bus_emit_properties_chan assert_return(bus, -EINVAL); assert_return(object_path_is_valid(path), -EINVAL); assert_return(interface_name_is_valid(interface), -EINVAL); - assert_return(BUS_IS_OPEN(bus->state), -ENOTCONN); assert_return(!bus_pid_changed(bus), -ECHILD); + if (!BUS_IS_OPEN(bus->state)) + return -ENOTCONN; /* A non-NULL but empty names list means nothing needs to be generated. A NULL list OTOH indicates that all properties @@ -2241,9 +2242,11 @@ _public_ int sd_bus_emit_properties_chan assert_return(bus, -EINVAL); assert_return(object_path_is_valid(path), -EINVAL); assert_return(interface_name_is_valid(interface), -EINVAL); - assert_return(BUS_IS_OPEN(bus->state), -ENOTCONN); assert_return(!bus_pid_changed(bus), -ECHILD); + if (!BUS_IS_OPEN(bus->state)) + return -ENOTCONN; + if (!name) return 0; @@ -2361,9 +2364,11 @@ _public_ int sd_bus_emit_interfaces_adde assert_return(bus, -EINVAL); assert_return(object_path_is_valid(path), -EINVAL); - assert_return(BUS_IS_OPEN(bus->state), -ENOTCONN); assert_return(!bus_pid_changed(bus), -ECHILD); + if (!BUS_IS_OPEN(bus->state)) + return -ENOTCONN; + if (strv_isempty(interfaces)) return 0; @@ -2421,9 +2426,11 @@ _public_ int sd_bus_emit_interfaces_adde assert_return(bus, -EINVAL); assert_return(object_path_is_valid(path), -EINVAL); - assert_return(BUS_IS_OPEN(bus->state), -ENOTCONN); assert_return(!bus_pid_changed(bus), -ECHILD); + if (!BUS_IS_OPEN(bus->state)) + return -ENOTCONN; + interfaces = strv_from_stdarg_alloca(interface); return sd_bus_emit_interfaces_added_strv(bus, path, interfaces); @@ -2435,9 +2442,11 @@ _public_ int sd_bus_emit_interfaces_remo assert_return(bus, -EINVAL); assert_return(object_path_is_valid(path), -EINVAL); - assert_return(BUS_IS_OPEN(bus->state), -ENOTCONN); assert_return(!bus_pid_changed(bus), -ECHILD); + if (!BUS_IS_OPEN(bus->state)) + return -ENOTCONN; + if (strv_isempty(interfaces)) return 0; @@ -2461,9 +2470,11 @@ _public_ int sd_bus_emit_interfaces_remo assert_return(bus, -EINVAL); assert_return(object_path_is_valid(path), -EINVAL); - assert_return(BUS_IS_OPEN(bus->state), -ENOTCONN); assert_return(!bus_pid_changed(bus), -ECHILD); + if (!BUS_IS_OPEN(bus->state)) + return -ENOTCONN; + interfaces = strv_from_stdarg_alloca(interface); return sd_bus_emit_interfaces_removed_strv(bus, path, interfaces); --- src/libsystemd/sd-bus/sd-bus.c +++ src/libsystemd/sd-bus/sd-bus.c 2014-03-28 12:19:27.146736146 +0000 @@ -1592,10 +1592,12 @@ static int bus_send_internal(sd_bus *bus int r; assert_return(bus, -EINVAL); - assert_return(BUS_IS_OPEN(bus->state), -ENOTCONN); assert_return(m, -EINVAL); assert_return(!bus_pid_changed(bus), -ECHILD); + if (!BUS_IS_OPEN(bus->state)) + return -ENOTCONN; + if (m->n_fds > 0) { r = sd_bus_can_send(bus, SD_BUS_TYPE_UNIX_FD); if (r < 0) @@ -1671,10 +1673,12 @@ _public_ int sd_bus_send_to(sd_bus *bus, int r; assert_return(bus, -EINVAL); - assert_return(BUS_IS_OPEN(bus->state), -ENOTCONN); assert_return(m, -EINVAL); assert_return(!bus_pid_changed(bus), -ECHILD); + if (!BUS_IS_OPEN(bus->state)) + return -ENOTCONN; + if (!streq_ptr(m->destination, destination)) { if (!destination) @@ -1726,13 +1730,15 @@ _public_ int sd_bus_call_async( int r; assert_return(bus, -EINVAL); - assert_return(BUS_IS_OPEN(bus->state), -ENOTCONN); assert_return(m, -EINVAL); assert_return(m->header->type == SD_BUS_MESSAGE_METHOD_CALL, -EINVAL); assert_return(!(m->header->flags & BUS_MESSAGE_NO_REPLY_EXPECTED), -EINVAL); assert_return(callback, -EINVAL); assert_return(!bus_pid_changed(bus), -ECHILD); + if (!BUS_IS_OPEN(bus->state)) + return -ENOTCONN; + r = hashmap_ensure_allocated(&bus->reply_callbacks, uint64_hash_func, uint64_compare_func); if (r < 0) return r; @@ -1839,13 +1845,15 @@ _public_ int sd_bus_call( int r; assert_return(bus, -EINVAL); - assert_return(BUS_IS_OPEN(bus->state), -ENOTCONN); assert_return(m, -EINVAL); assert_return(m->header->type == SD_BUS_MESSAGE_METHOD_CALL, -EINVAL); assert_return(!(m->header->flags & BUS_MESSAGE_NO_REPLY_EXPECTED), -EINVAL); assert_return(!bus_error_is_dirty(error), -EINVAL); assert_return(!bus_pid_changed(bus), -ECHILD); + if (!BUS_IS_OPEN(bus->state)) + return -ENOTCONN; + r = bus_ensure_running(bus); if (r < 0) return r; @@ -1971,9 +1979,11 @@ _public_ int sd_bus_get_events(sd_bus *b int flags = 0; assert_return(bus, -EINVAL); - assert_return(BUS_IS_OPEN(bus->state) || bus->state == BUS_CLOSING, -ENOTCONN); assert_return(!bus_pid_changed(bus), -ECHILD); + if (!BUS_IS_OPEN(bus->state) && bus->state != BUS_CLOSING) + return -ENOTCONN; + if (bus->state == BUS_OPENING) flags |= POLLOUT; else if (bus->state == BUS_AUTHENTICATING) { @@ -1998,9 +2008,11 @@ _public_ int sd_bus_get_timeout(sd_bus * assert_return(bus, -EINVAL); assert_return(timeout_usec, -EINVAL); - assert_return(BUS_IS_OPEN(bus->state) || bus->state == BUS_CLOSING, -ENOTCONN); assert_return(!bus_pid_changed(bus), -ECHILD); + if (!BUS_IS_OPEN(bus->state) && bus->state != BUS_CLOSING) + return -ENOTCONN; + if (bus->state == BUS_CLOSING) { *timeout_usec = 0; return 1; @@ -2510,7 +2522,8 @@ static int bus_poll(sd_bus *bus, bool ne if (bus->state == BUS_CLOSING) return 1; - assert_return(BUS_IS_OPEN(bus->state), -ENOTCONN); + if (!BUS_IS_OPEN(bus->state)) + return -ENOTCONN; e = sd_bus_get_events(bus); if (e < 0) @@ -2565,7 +2578,8 @@ _public_ int sd_bus_wait(sd_bus *bus, ui if (bus->state == BUS_CLOSING) return 0; - assert_return(BUS_IS_OPEN(bus->state) , -ENOTCONN); + if (!BUS_IS_OPEN(bus->state)) + return -ENOTCONN; if (bus->rqueue_size > 0) return 0; @@ -2582,7 +2596,8 @@ _public_ int sd_bus_flush(sd_bus *bus) { if (bus->state == BUS_CLOSING) return 0; - assert_return(BUS_IS_OPEN(bus->state), -ENOTCONN); + if (!BUS_IS_OPEN(bus->state)) + return -ENOTCONN; r = bus_ensure_running(bus); if (r < 0) @@ -3058,9 +3073,13 @@ _public_ int sd_bus_get_peer_creds(sd_bu assert_return(bus, -EINVAL); assert_return(mask <= _SD_BUS_CREDS_ALL, -ENOTSUP); assert_return(ret, -EINVAL); - assert_return(BUS_IS_OPEN(bus->state), -ENOTCONN); assert_return(!bus_pid_changed(bus), -ECHILD); - assert_return(!bus->is_kernel, -ENOTSUP); + + if (!bus->is_kernel) + return -ENOTSUP; + + if (!BUS_IS_OPEN(bus->state)) + return -ENOTCONN; if (!bus->ucred_valid && !isempty(bus->label)) return -ENODATA; @@ -3099,9 +3118,13 @@ _public_ int sd_bus_try_close(sd_bus *bu int r; assert_return(bus, -EINVAL); - assert_return(BUS_IS_OPEN(bus->state), -ENOTCONN); assert_return(!bus_pid_changed(bus), -ECHILD); - assert_return(bus->is_kernel, -ENOTSUP); + + if (!bus->is_kernel) + return -ENOTSUP; + + if (!BUS_IS_OPEN(bus->state)) + return -ENOTCONN; if (bus->rqueue_size > 0) return -EBUSY;