bus: rework message handlers to always take an error argument

Message handler callbacks can be simplified drastically if the
dispatcher automatically replies to method calls if errors are returned.

Thus: add an sd_bus_error argument to all message handlers. When we
dispatch a message handler and it returns negative or a set sd_bus_error
we send this as message error back to the client. This means errors
returned by handlers by default are given back to clients instead of
rippling all the way up to the event loop, which is desirable to make
things robust.

As a side-effect we can now easily turn the SELinux checks into normal
function calls, since the method call dispatcher will generate the right
error replies automatically now.

Also, make sure we always pass the error structure to all property and
method handlers as last argument to follow the usual style of passing
variables for return values as last argument.

1 files changed, 28 insertions, 24 deletions

diff --git a/src/locale/localed.c b/src/locale/localed.c
index 7bf80fbf2b..7ef4d13079 100644
--- a/src/locale/localed.c
+++ b/src/locale/localed.c
@@ -787,8 +787,15 @@ static int x11_convert_to_vconsole(Context *c, sd_bus *bus) {
         return 0;
 }
 
-static int property_get_locale(sd_bus *bus, const char *path, const char *interface,
-                               const char *property, sd_bus_message *reply, sd_bus_error *error, void *userdata) {
+static int property_get_locale(
+                sd_bus *bus,
+                const char *path,
+                const char *interface,
+                const char *property,
+                sd_bus_message *reply,
+                void *userdata,
+                sd_bus_error *error) {
+
         Context *c = userdata;
         _cleanup_strv_free_ char **l = NULL;
         int p, q;
@@ -812,9 +819,8 @@ static int property_get_locale(sd_bus *bus, const char *path, const char *interf
         return sd_bus_message_append_strv(reply, l);
 }
 
-static int method_set_locale(sd_bus *bus, sd_bus_message *m, void *userdata) {
+static int method_set_locale(sd_bus *bus, sd_bus_message *m, void *userdata, sd_bus_error *error) {
         Context *c = userdata;
-        _cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL;
         _cleanup_strv_free_ char **l = NULL;
         char **i;
         int interactive;
@@ -825,11 +831,11 @@ static int method_set_locale(sd_bus *bus, sd_bus_message *m, void *userdata) {
 
         r = bus_message_read_strv_extend(m, &l);
         if (r < 0)
-                return sd_bus_reply_method_errno(m, r, NULL);
+                return r;
 
         r = sd_bus_message_read_basic(m, 'b', &interactive);
         if (r < 0)
-                return sd_bus_reply_method_errno(m, r, NULL);
+                return r;
 
         /* Check whether a variable changed and if so valid */
         STRV_FOREACH(i, l) {
@@ -853,7 +859,7 @@ static int method_set_locale(sd_bus *bus, sd_bus_message *m, void *userdata) {
                 }
 
                 if (!valid)
-                        sd_bus_reply_method_errorf(m, SD_BUS_ERROR_INVALID_ARGS, "Invalid Locale data.");
+                        return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid Locale data.");
         }
 
         /* Check whether a variable is unset */
@@ -868,9 +874,9 @@ static int method_set_locale(sd_bus *bus, sd_bus_message *m, void *userdata) {
         if (modified) {
                 r = bus_verify_polkit_async(bus, &c->polkit_registry, m,
                                             "org.freedesktop.locale1.set-locale", interactive,
-                                            &error, method_set_locale, c);
+                                            error, method_set_locale, c);
                 if (r < 0)
-                        return sd_bus_reply_method_errno(m, r, &error);
+                        return r;
                 if (r == 0)
                         return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */
 
@@ -905,7 +911,7 @@ static int method_set_locale(sd_bus *bus, sd_bus_message *m, void *userdata) {
                 r = locale_write_data(c);
                 if (r < 0) {
                         log_error("Failed to set locale: %s", strerror(-r));
-                        return sd_bus_reply_method_errnof(m, r, "Failed to set locale: %s", strerror(-r));
+                        return sd_bus_error_set_errnof(error, r, "Failed to set locale: %s", strerror(-r));
                 }
 
                 locale_update_system_manager(c, bus);
@@ -921,16 +927,15 @@ static int method_set_locale(sd_bus *bus, sd_bus_message *m, void *userdata) {
         return sd_bus_reply_method_return(m, NULL);
 }
 
-static int method_set_vc_keyboard(sd_bus *bus, sd_bus_message *m, void *userdata) {
+static int method_set_vc_keyboard(sd_bus *bus, sd_bus_message *m, void *userdata, sd_bus_error *error) {
         Context *c = userdata;
-        _cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL;
         const char *keymap, *keymap_toggle;
         int convert, interactive;
         int r;
 
         r = sd_bus_message_read(m, "ssbb", &keymap, &keymap_toggle, &convert, &interactive);
         if (r < 0)
-                return sd_bus_reply_method_errno(m, r, NULL);
+                return r;
 
         if (isempty(keymap))
                 keymap = NULL;
@@ -943,13 +948,13 @@ static int method_set_vc_keyboard(sd_bus *bus, sd_bus_message *m, void *userdata
 
                 if ((keymap && (!filename_is_safe(keymap) || !string_is_safe(keymap))) ||
                     (keymap_toggle && (!filename_is_safe(keymap_toggle) || !string_is_safe(keymap_toggle))))
-                        return sd_bus_reply_method_errnof(m, r, "Received invalid keymap data: %s", -EINVAL);
+                        return sd_bus_error_set_errnof(error, r, "Received invalid keymap data: %s", -EINVAL);
 
                 r = bus_verify_polkit_async(bus, &c->polkit_registry, m,
                                 "org.freedesktop.locale1.set-keyboard",
-                                interactive, &error, method_set_vc_keyboard, c);
+                                interactive, error, method_set_vc_keyboard, c);
                 if (r < 0)
-                        return sd_bus_reply_method_errno(m, r, &error);
+                        return r;
                 if (r == 0)
                         return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */
 
@@ -960,7 +965,7 @@ static int method_set_vc_keyboard(sd_bus *bus, sd_bus_message *m, void *userdata
                 r = vconsole_write_data(c);
                 if (r < 0) {
                         log_error("Failed to set virtual console keymap: %s", strerror(-r));
-                        return sd_bus_reply_method_errnof(m, r, "Failed to set virtual console keymap: %s", strerror(-r));
+                        return sd_bus_error_set_errnof(error, r, "Failed to set virtual console keymap: %s", strerror(-r));
                 }
 
                 log_info("Changed virtual console keymap to '%s'", strempty(c->vc_keymap));
@@ -984,16 +989,15 @@ static int method_set_vc_keyboard(sd_bus *bus, sd_bus_message *m, void *userdata
         return sd_bus_reply_method_return(m, NULL);
 }
 
-static int method_set_x11_keyboard(sd_bus *bus, sd_bus_message *m, void *userdata) {
+static int method_set_x11_keyboard(sd_bus *bus, sd_bus_message *m, void *userdata, sd_bus_error *error) {
         Context *c = userdata;
-        _cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL;
         const char *layout, *model, *variant, *options;
         int convert, interactive;
         int r;
 
         r = sd_bus_message_read(m, "ssssbb", &layout, &model, &variant, &options, &convert, &interactive);
         if (r < 0)
-                return sd_bus_reply_method_errno(m, r, NULL);
+                return r;
 
         if (isempty(layout))
                 layout = NULL;
@@ -1016,13 +1020,13 @@ static int method_set_x11_keyboard(sd_bus *bus, sd_bus_message *m, void *userdat
                     (model && !string_is_safe(model)) ||
                     (variant && !string_is_safe(variant)) ||
                     (options && !string_is_safe(options)))
-                        return sd_bus_reply_method_errnof(m, r, "Received invalid keyboard data: %s", -EINVAL);
+                        return sd_bus_error_set_errnof(error, r, "Received invalid keyboard data: %s", -EINVAL);
 
                 r = bus_verify_polkit_async(bus, &c->polkit_registry, m,
                                 "org.freedesktop.locale1.set-keyboard",
-                                interactive, &error, method_set_x11_keyboard, c);
+                                interactive, error, method_set_x11_keyboard, c);
                 if (r < 0)
-                        return sd_bus_reply_method_errno(m, r, &error);
+                        return r;
                 if (r == 0)
                         return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */
 
@@ -1035,7 +1039,7 @@ static int method_set_x11_keyboard(sd_bus *bus, sd_bus_message *m, void *userdat
                 r = write_data_x11(c);
                 if (r < 0) {
                         log_error("Failed to set X11 keyboard layout: %s", strerror(-r));
-                        return sd_bus_reply_method_errnof(m, r, "Failed to set X11 keyboard layout: %s", strerror(-r));
+                        return sd_bus_error_set_errnof(error, r, "Failed to set X11 keyboard layout: %s", strerror(-r));
                 }
 
                 log_info("Changed X11 keyboard layout to '%s'", strempty(c->x11_layout));