From 05c754bc7fe14e71721af6eb16194b9ad0690fb5 Mon Sep 17 00:00:00 2001
From: Mike Yuan <me@yhndnzj.com>
Date: Wed, 10 Jul 2024 22:23:37 +0200
Subject: [PATCH] exec-util: modernize exec_command_flags_{to,from}_strv

- Rename ret params following our coding style
- Use assertion where appropriate
- Use BIT_FOREACH()
---
 src/shared/exec-util.c    | 49 ++++++++++++++++++---------------------
 src/shared/exec-util.h    |  4 ++--
 src/test/test-exec-util.c | 22 ++++++------------
 3 files changed, 32 insertions(+), 43 deletions(-)

diff --git a/src/shared/exec-util.c b/src/shared/exec-util.c
index 996edbf997a..5408be5328b 100644
--- a/src/shared/exec-util.c
+++ b/src/shared/exec-util.c
@@ -8,6 +8,7 @@
 #include <stdio.h>
 
 #include "alloc-util.h"
+#include "bitfield.h"
 #include "conf-files.h"
 #include "env-file.h"
 #include "env-util.h"
@@ -425,46 +426,42 @@ static int gather_environment_consume(int fd, void *arg) {
         return r;
 }
 
-int exec_command_flags_from_strv(char **ex_opts, ExecCommandFlags *flags) {
-        ExecCommandFlags ex_flag, ret_flags = 0;
+int exec_command_flags_from_strv(char * const *ex_opts, ExecCommandFlags *ret) {
+        ExecCommandFlags flags = 0;
 
-        assert(flags);
+        assert(ret);
 
         STRV_FOREACH(opt, ex_opts) {
-                ex_flag = exec_command_flags_from_string(*opt);
-                if (ex_flag < 0)
-                        return ex_flag;
-                ret_flags |= ex_flag;
+                ExecCommandFlags fl = exec_command_flags_from_string(*opt);
+                if (fl < 0)
+                        return fl;
+
+                flags |= fl;
         }
 
-        *flags = ret_flags;
+        *ret = flags;
 
         return 0;
 }
 
-int exec_command_flags_to_strv(ExecCommandFlags flags, char ***ex_opts) {
-        _cleanup_strv_free_ char **ret_opts = NULL;
-        ExecCommandFlags it = flags;
-        const char *str;
+int exec_command_flags_to_strv(ExecCommandFlags flags, char ***ret) {
+        _cleanup_strv_free_ char **opts = NULL;
         int r;
 
-        assert(ex_opts);
+        assert(flags >= 0);
+        assert(ret);
 
-        if (flags < 0)
-                return flags;
+        BIT_FOREACH(i, flags) {
+                const char *s = exec_command_flags_to_string(1 << i);
+                if (!s)
+                        return -EINVAL;
 
-        for (unsigned i = 0; it != 0; it &= ~(1 << i), i++)
-                if (FLAGS_SET(flags, (1 << i))) {
-                        str = exec_command_flags_to_string(1 << i);
-                        if (!str)
-                                return -EINVAL;
+                r = strv_extend(&opts, s);
+                if (r < 0)
+                        return r;
+        }
 
-                        r = strv_extend(&ret_opts, str);
-                        if (r < 0)
-                                return r;
-                }
-
-        *ex_opts = TAKE_PTR(ret_opts);
+        *ret = TAKE_PTR(opts);
 
         return 0;
 }
diff --git a/src/shared/exec-util.h b/src/shared/exec-util.h
index b99336ee3be..6c2d6b19cde 100644
--- a/src/shared/exec-util.h
+++ b/src/shared/exec-util.h
@@ -51,8 +51,8 @@ int execute_directories(
                 char *envp[],
                 ExecDirFlags flags);
 
-int exec_command_flags_from_strv(char **ex_opts, ExecCommandFlags *flags);
-int exec_command_flags_to_strv(ExecCommandFlags flags, char ***ex_opts);
+int exec_command_flags_from_strv(char * const *ex_opts, ExecCommandFlags *ret);
+int exec_command_flags_to_strv(ExecCommandFlags flags, char ***ret);
 
 extern const gather_stdout_callback_t gather_environment[_STDOUT_CONSUME_MAX];
 
diff --git a/src/test/test-exec-util.c b/src/test/test-exec-util.c
index 533c988a472..301e02db0b8 100644
--- a/src/test/test-exec-util.c
+++ b/src/test/test-exec-util.c
@@ -429,27 +429,19 @@ TEST(exec_command_flags_from_strv) {
 }
 
 TEST(exec_command_flags_to_strv) {
-        _cleanup_strv_free_ char **opts = NULL, **empty_opts = NULL, **invalid_opts = NULL;
-        ExecCommandFlags flags = 0;
-        int r;
+        _cleanup_strv_free_ char **opts = NULL;
 
-        flags |= (EXEC_COMMAND_AMBIENT_MAGIC|EXEC_COMMAND_NO_ENV_EXPAND|EXEC_COMMAND_IGNORE_FAILURE);
-
-        r = exec_command_flags_to_strv(flags, &opts);
-
-        assert_se(r == 0);
+        ASSERT_OK(exec_command_flags_to_strv(EXEC_COMMAND_AMBIENT_MAGIC|EXEC_COMMAND_NO_ENV_EXPAND|EXEC_COMMAND_IGNORE_FAILURE, &opts));
         assert_se(strv_equal(opts, STRV_MAKE("ignore-failure", "ambient", "no-env-expand")));
 
-        r = exec_command_flags_to_strv(0, &empty_opts);
+        opts = strv_free(opts);
 
-        assert_se(r == 0);
-        assert_se(strv_equal(empty_opts, STRV_MAKE_EMPTY));
+        ASSERT_OK(exec_command_flags_to_strv(0, &opts));
+        assert_se(strv_isempty(opts));
 
-        flags = _EXEC_COMMAND_FLAGS_INVALID;
+        opts = strv_free(opts);
 
-        r = exec_command_flags_to_strv(flags, &invalid_opts);
-
-        assert_se(r == -EINVAL);
+        ASSERT_ERROR(exec_command_flags_to_strv(1U << 20, &opts), EINVAL);
 }
 
 DEFINE_TEST_MAIN(LOG_DEBUG);