]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
homectl: split out inspect_home() and authenticate_home()
authorZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Tue, 19 Nov 2024 15:50:44 +0000 (16:50 +0100)
committerZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Tue, 25 Mar 2025 09:25:09 +0000 (10:25 +0100)
mangle_user_list() was doing a microoptimization of avoiding of copying of a
single string by constructing the strv object manually. This seems like more
trouble than it's worth, considering that this is called once in the program's
life.

Rework the code to have inspect_home() and authenticate_home() that handle a
single home name and call them in a loop from the outer function.

src/home/homectl.c

index c4c1ad9761f5d71dd202a1e006ffdcccab356863..acceadf176eae3262f97d42818ba9b36c4717f48 100644 (file)
@@ -710,114 +710,106 @@ static void dump_home_record(UserRecord *hr) {
         }
 }
 
-static char **mangle_user_list(char **list, char ***ret_allocated) {
-        _cleanup_free_ char *myself = NULL;
-        char **l;
+static int inspect_home(sd_bus *bus, const char *name) {
+        _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
+        _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL;
+        _cleanup_(sd_json_variant_unrefp) sd_json_variant *v = NULL;
+        _cleanup_(user_record_unrefp) UserRecord *hr = NULL;
+        const char *json;
+        int incomplete;
+        uid_t uid;
+        int r;
 
-        if (!strv_isempty(list)) {
-                *ret_allocated = NULL;
-                return list;
-        }
+        r = parse_uid(name, &uid);
+        if (r < 0)
+                r = bus_call_method(bus, bus_mgr, "GetUserRecordByName", &error, &reply, "s", name);
+        else
+                r = bus_call_method(bus, bus_mgr, "GetUserRecordByUID", &error, &reply, "u", (uint32_t) uid);
+        if (r < 0)
+                return log_error_errno(r, "Failed to inspect home: %s", bus_error_message(&error, r));
 
-        myself = getusername_malloc();
-        if (!myself)
-                return NULL;
+        r = sd_bus_message_read(reply, "sbo", &json, &incomplete, NULL);
+        if (r < 0)
+                return bus_log_parse_error(r);
 
-        l = new(char*, 2);
-        if (!l)
-                return NULL;
+        r = sd_json_parse(json, SD_JSON_PARSE_SENSITIVE, &v, NULL, NULL);
+        if (r < 0)
+                return log_error_errno(r, "Failed to parse JSON identity: %m");
 
-        l[0] = TAKE_PTR(myself);
-        l[1] = NULL;
+        hr = user_record_new();
+        if (!hr)
+                return log_oom();
+
+        r = user_record_load(hr, v, USER_RECORD_LOAD_REFUSE_SECRET|USER_RECORD_LOG|USER_RECORD_PERMISSIVE);
+        if (r < 0)
+                return r;
 
-        *ret_allocated = l;
-        return l;
+        hr->incomplete = incomplete;
+        dump_home_record(hr);
+        return 0;
 }
 
-static int inspect_home(int argc, char *argv[], void *userdata) {
+static int inspect_homes(int argc, char *argv[], void *userdata) {
         _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL;
-        _cleanup_strv_free_ char **mangled_list = NULL;
-        int r, ret = 0;
-        char **items;
-
-        pager_open(arg_pager_flags);
+        int r;
 
         r = acquire_bus(&bus);
         if (r < 0)
                 return r;
 
-        items = mangle_user_list(strv_skip(argv, 1), &mangled_list);
-        if (!items)
-                return log_oom();
+        pager_open(arg_pager_flags);
 
-        STRV_FOREACH(i, items) {
-                _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
-                _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL;
-                _cleanup_(sd_json_variant_unrefp) sd_json_variant *v = NULL;
-                _cleanup_(user_record_unrefp) UserRecord *hr = NULL;
-                const char *json;
-                int incomplete;
-                uid_t uid;
+        char **args = strv_skip(argv, 1);
+        if (strv_isempty(args)) {
+                _cleanup_free_ char *myself = getusername_malloc();
+                if (!myself)
+                        return log_oom();
 
-                r = parse_uid(*i, &uid);
-                if (r < 0)
-                        r = bus_call_method(bus, bus_mgr, "GetUserRecordByName", &error, &reply, "s", *i);
-                else
-                        r = bus_call_method(bus, bus_mgr, "GetUserRecordByUID", &error, &reply, "u", (uint32_t) uid);
-                if (r < 0) {
-                        log_error_errno(r, "Failed to inspect home: %s", bus_error_message(&error, r));
-                        if (ret == 0)
-                                ret = r;
+                return inspect_home(bus, myself);
+        } else {
+                STRV_FOREACH(arg, args)
+                        RET_GATHER(r, inspect_home(bus, *arg));
+                return r;
+        }
+}
 
-                        continue;
-                }
+static int authenticate_home(sd_bus *bus, const char *name) {
+        _cleanup_(user_record_unrefp) UserRecord *secret = NULL;
+        int r;
 
-                r = sd_bus_message_read(reply, "sbo", &json, &incomplete, NULL);
-                if (r < 0) {
-                        bus_log_parse_error(r);
-                        if (ret == 0)
-                                ret = r;
+        r = acquire_passed_secrets(name, &secret);
+        if (r < 0)
+                return r;
 
-                        continue;
-                }
+        for (;;) {
+                _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
+                _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL;
 
-                r = sd_json_parse(json, SD_JSON_PARSE_SENSITIVE, &v, NULL, NULL);
-                if (r < 0) {
-                        log_error_errno(r, "Failed to parse JSON identity: %m");
-                        if (ret == 0)
-                                ret = r;
+                r = bus_message_new_method_call(bus, &m, bus_mgr, "AuthenticateHome");
+                if (r < 0)
+                        return bus_log_create_error(r);
 
-                        continue;
-                }
+                r = sd_bus_message_append(m, "s", name);
+                if (r < 0)
+                        return bus_log_create_error(r);
 
-                hr = user_record_new();
-                if (!hr)
-                        return log_oom();
+                r = bus_message_append_secret(m, secret);
+                if (r < 0)
+                        return bus_log_create_error(r);
 
-                r = user_record_load(hr, v, USER_RECORD_LOAD_REFUSE_SECRET|USER_RECORD_LOG|USER_RECORD_PERMISSIVE);
+                r = sd_bus_call(bus, m, HOME_SLOW_BUS_CALL_TIMEOUT_USEC, &error, NULL);
                 if (r < 0) {
-                        if (ret == 0)
-                                ret = r;
-
-                        continue;
+                        r = handle_generic_user_record_error(name, secret, &error, r, false);
+                        if (r >= 0)
+                                continue;
                 }
-
-                hr->incomplete = incomplete;
-                dump_home_record(hr);
+                return r;
         }
-
-        return ret;
 }
 
-static int authenticate_home(int argc, char *argv[], void *userdata) {
+static int authenticate_homes(int argc, char *argv[], void *userdata) {
         _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL;
-        _cleanup_strv_free_ char **mangled_list = NULL;
-        int r, ret = 0;
-        char **items;
-
-        items = mangle_user_list(strv_skip(argv, 1), &mangled_list);
-        if (!items)
-                return log_oom();
+        int r;
 
         r = acquire_bus(&bus);
         if (r < 0)
@@ -825,44 +817,19 @@ static int authenticate_home(int argc, char *argv[], void *userdata) {
 
         (void) polkit_agent_open_if_enabled(arg_transport, arg_ask_password);
 
-        STRV_FOREACH(i, items) {
-                _cleanup_(user_record_unrefp) UserRecord *secret = NULL;
-
-                r = acquire_passed_secrets(*i, &secret);
-                if (r < 0)
-                        return r;
-
-                for (;;) {
-                        _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
-                        _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL;
-
-                        r = bus_message_new_method_call(bus, &m, bus_mgr, "AuthenticateHome");
-                        if (r < 0)
-                                return bus_log_create_error(r);
-
-                        r = sd_bus_message_append(m, "s", *i);
-                        if (r < 0)
-                                return bus_log_create_error(r);
-
-                        r = bus_message_append_secret(m, secret);
-                        if (r < 0)
-                                return bus_log_create_error(r);
+        char **args = strv_skip(argv, 1);
+        if (strv_isempty(args)) {
+                _cleanup_free_ char *myself = getusername_malloc();
+                if (!myself)
+                        return log_oom();
 
-                        r = sd_bus_call(bus, m, HOME_SLOW_BUS_CALL_TIMEOUT_USEC, &error, NULL);
-                        if (r < 0) {
-                                r = handle_generic_user_record_error(*i, secret, &error, r, false);
-                                if (r < 0) {
-                                        if (ret == 0)
-                                                ret = r;
+                return authenticate_home(bus, myself);
+        } else {
+                STRV_FOREACH(arg, args)
+                        RET_GATHER(r, authenticate_home(bus, *arg));
 
-                                        break;
-                                }
-                        } else
-                                break;
-                }
+                return r;
         }
-
-        return ret;
 }
 
 static int update_last_change(sd_json_variant **v, bool with_password, bool override) {
@@ -5477,8 +5444,8 @@ static int run(int argc, char *argv[]) {
                 { "list",               VERB_ANY, 1,        VERB_DEFAULT, list_homes               },
                 { "activate",           2,        VERB_ANY, 0,            activate_home            },
                 { "deactivate",         2,        VERB_ANY, 0,            deactivate_home          },
-                { "inspect",            VERB_ANY, VERB_ANY, 0,            inspect_home             },
-                { "authenticate",       VERB_ANY, VERB_ANY, 0,            authenticate_home        },
+                { "inspect",            VERB_ANY, VERB_ANY, 0,            inspect_homes            },
+                { "authenticate",       VERB_ANY, VERB_ANY, 0,            authenticate_homes       },
                 { "create",             VERB_ANY, 2,        0,            create_home              },
                 { "adopt",              VERB_ANY, VERB_ANY, 0,            verb_adopt_home          },
                 { "register",           VERB_ANY, VERB_ANY, 0,            verb_register_home       },