From: Mike Yuan <me@yhndnzj.com>
Date: Sun, 7 Jul 2024 15:21:08 +0000 (+0200)
Subject: edit-util: clean up run_editor() a bit
X-Git-Tag: v257-rc1~712^2~3
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=7a729f876b2494e2f1d1def73b828a9b70dd4723;p=thirdparty%2Fsystemd.git

edit-util: clean up run_editor() a bit

- Add missing assertions
- Close all fds before spawning editor
- Use FOREACH_STRING() + empty_to_null() where appropriate
  Note that this slightly changes the behavior, in that
  empty envvars would be treated as unset and we'd try
  the next candidate. But the new behavior is better IMO.
---

diff --git a/src/shared/edit-util.c b/src/shared/edit-util.c
index cfb2828f4e3..2d0129eda27 100644
--- a/src/shared/edit-util.c
+++ b/src/shared/edit-util.c
@@ -236,23 +236,22 @@ static int run_editor_child(const EditFileContext *context) {
         const char *editor;
         int r;
 
+        assert(context);
+        assert(context->n_files >= 1);
+
         /* SYSTEMD_EDITOR takes precedence over EDITOR which takes precedence over VISUAL.
          * If neither SYSTEMD_EDITOR nor EDITOR nor VISUAL are present, we try to execute
          * well known editors. */
-        editor = getenv("SYSTEMD_EDITOR");
-        if (!editor)
-                editor = getenv("EDITOR");
-        if (!editor)
-                editor = getenv("VISUAL");
-
-        if (!isempty(editor)) {
-                _cleanup_strv_free_ char **editor_args = NULL;
+        FOREACH_STRING(e, "SYSTEMD_EDITOR", "EDITOR", "VISUAL") {
+                editor = empty_to_null(getenv(e));
+                if (editor)
+                        break;
+        }
 
-                editor_args = strv_split(editor, WHITESPACE);
-                if (!editor_args)
+        if (editor) {
+                args = strv_split(editor, WHITESPACE);
+                if (!args)
                         return log_oom();
-
-                args = TAKE_PTR(editor_args);
         }
 
         if (context->n_files == 1 && context->files[0].line > 1) {
@@ -268,7 +267,7 @@ static int run_editor_child(const EditFileContext *context) {
                         return log_oom();
         }
 
-        if (!isempty(editor))
+        if (editor)
                 execvp(args[0], (char* const*) args);
 
         bool prepended = false;
@@ -298,7 +297,7 @@ static int run_editor(const EditFileContext *context) {
 
         assert(context);
 
-        r = safe_fork("(editor)", FORK_RESET_SIGNALS|FORK_DEATHSIG_SIGTERM|FORK_RLIMIT_NOFILE_SAFE|FORK_LOG|FORK_WAIT, NULL);
+        r = safe_fork("(editor)", FORK_RESET_SIGNALS|FORK_DEATHSIG_SIGTERM|FORK_RLIMIT_NOFILE_SAFE|FORK_CLOSE_ALL_FDS|FORK_REOPEN_LOG|FORK_LOG|FORK_WAIT, NULL);
         if (r < 0)
                 return r;
         if (r == 0) { /* Child */