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.

1 files changed, 13 insertions, 14 deletions

diff --git a/src/shared/edit-util.c b/src/shared/edit-util.c
index cfb2828f4e..2d0129eda2 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 */