unit_control: don't quit on errors 72/211472/1
authorAdrian Szyndela <adrian.s@samsung.com>
Mon, 5 Aug 2019 12:17:52 +0000 (14:17 +0200)
committerAdrian Szyndela <adrian.s@samsung.com>
Mon, 5 Aug 2019 12:24:13 +0000 (14:24 +0200)
This makes unit_control not return errors to systemd's source_dispatch.
That is, errors are sent back to clients as replies through D-Bus,
but the unit_control module itself does not report errors
to the dispatch, because the dispatch would disable the unit_control.
Such behaviour would make subsequent clients reaching their timeouts
instead of getting meaningful responses, such as "Access denied",
or "This is not supported, stupid!".

Change-Id: I1f624c3d7bb0ed31a0310371946c341fd3ad7feb

src/decision_makers/unit_control_dm.c

index 9e5714a3e26d983e36d7437c9a194583acdc7045..a2c44fd4663b5e65fbc157fef1e8c2d1d26729cd 100644 (file)
@@ -415,12 +415,11 @@ static int unit_control_make_decision(struct epc_event_handler *handler)
        struct unit_control_decision_maker *dm = container_of(handler,
                        struct unit_control_decision_maker, eh);
        struct unit_control_event *ev = to_unit_control_event(event);
-       int ret = 0;
        bool matched = false;
        int error_code = 0;
        struct whitelist_entry *w;
        _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
-       char *service_name;
+       _cleanup_free_ char *service_name = NULL;
 
        /* cannot use start with wildcards */
        if (!strcmp(ev->method, "start") && strstr(ev->unit, "@.")) {
@@ -444,14 +443,14 @@ static int unit_control_make_decision(struct epc_event_handler *handler)
                log_debug("execute action %s on pattern %s", ev->method, service_name);
 
                if (!strcmp(ev->method, "start") || !is_wildcard(service_name)) /* no need to query when starting or for single unit*/
-                       ret = unit_control_execute(service_name, ev->method, event);
+                       error_code = unit_control_execute(service_name, ev->method, event);
                else
-                       ret = unit_control_query_and_execute(service_name, ev->method, event);
-               if (ret < 0) {
-                       log_error_errno(ret, "action %s on unit %s failed (%m)", ev->method, service_name);
-                       error_code = ret;
+                       error_code = unit_control_query_and_execute(service_name, ev->method, event);
+
+               if (error_code < 0) {
+                       log_error_errno(error_code, "action %s on unit %s failed (%m)", ev->method, service_name);
+                       goto cleanup;
                }
-               free(service_name);
        }
 
        if (matched != true)
@@ -460,14 +459,11 @@ static int unit_control_make_decision(struct epc_event_handler *handler)
 cleanup:
        if (error_code != 0) {
                sd_bus_error_set_errno(&error, error_code);
-               ret = sd_bus_reply_method_error(ev->m, &error);
+               sd_bus_reply_method_error(ev->m, &error);
                sd_bus_message_unref(ev->m);
-               ret = error_code;
-       } else {
-               ret = 0;
        }
        epc_event_unref(event);
-       return ret;
+       return 0;
 }
 
 static void cleanup_whitelist_entry(struct whitelist_entry *w)