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 cfb2828f4e3b4c7caba8c3218d492cac8b23841e..2d0129eda2791f478a6aaceadb94a98eddd5c671 100644 (file)
--- 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 */