edit-util: introduce EditFileContext

This is a rather large change which moves
the add and install logic into edit-util.

We store an EditFile array and the number of
elements, along with the edit markers used in
temporary files and whether to remove the parent
directories of the target files if they're empty
in an EditFileContext object.

Call edit_files_add() to add an file to edit,
and do_edit_files_and_install() to do the actual
editing (through create_edit_temp_file(),
run_editor() and trim_edit_markers()).
After that, edit_file_context_done() can be used
to destroy the object.

3 files changed, 181 insertions, 114 deletions

diff --git a/src/shared/edit-util.c b/src/shared/edit-util.c
index 2ad81e7e09..38a1dfc089 100644
--- a/src/shared/edit-util.c
+++ b/src/shared/edit-util.c
@@ -18,19 +18,93 @@
 #include "strv.h"
 #include "tmpfile-util.h"
 
-void edit_file_free_all(EditFile **f) {
-        if (!f || !*f)
-                return;
+void edit_file_context_done(EditFileContext *context) {
+        int r;
+
+        assert(context);
+
+        FOREACH_ARRAY(i, context->files, context->n_files) {
+                if (i->temp) {
+                        (void) unlink(i->temp);
+                        free(i->temp);
+                }
+
+                if (context->remove_parent) {
+                        _cleanup_free_ char *parent = NULL;
+
+                        r = path_extract_directory(i->path, &parent);
+                        if (r < 0)
+                                log_debug_errno(r, "Failed to extract directory from '%s', ignoring: %m", i->path);
+
+                        /* No need to check if the dir is empty, rmdir does nothing if it is not the case. */
+                        (void) rmdir(parent);
+                }
 
-        for (EditFile *i = *f; i->path; i++) {
                 free(i->path);
-                free(i->tmp);
+                free(i->original_path);
+                strv_free(i->comment_paths);
         }
 
-        free(*f);
+        context->files = mfree(context->files);
+        context->n_files = 0;
+}
+
+bool edit_files_contains(const EditFileContext *context, const char *path) {
+        assert(context);
+        assert(path);
+
+        FOREACH_ARRAY(i, context->files, context->n_files)
+                if (streq(i->path, path))
+                        return true;
+
+        return false;
 }
 
-int create_edit_temp_file(
+int edit_files_add(
+                EditFileContext *context,
+                const char *path,
+                const char *original_path,
+                char * const *comment_paths) {
+
+        _cleanup_free_ char *new_path = NULL, *new_original_path = NULL;
+        _cleanup_strv_free_ char **new_comment_paths = NULL;
+
+        assert(context);
+        assert(path);
+
+        if (edit_files_contains(context, path))
+                return 0;
+
+        if (!GREEDY_REALLOC0(context->files, context->n_files + 2))
+                return log_oom();
+
+        new_path = strdup(path);
+        if (!new_path)
+                return log_oom();
+
+        if (original_path) {
+                new_original_path = strdup(original_path);
+                if (!new_original_path)
+                        return log_oom();
+        }
+
+        if (comment_paths) {
+                new_comment_paths = strv_copy(comment_paths);
+                if (!new_comment_paths)
+                        return log_oom();
+        }
+
+        context->files[context->n_files] = (EditFile) {
+                .path = TAKE_PTR(new_path),
+                .original_path = TAKE_PTR(new_original_path),
+                .comment_paths = TAKE_PTR(new_comment_paths),
+        };
+        context->n_files++;
+
+        return 1;
+}
+
+static int create_edit_temp_file(
                 const char *target_path,
                 const char *original_path,
                 char * const *comment_paths,
@@ -145,10 +219,10 @@ int create_edit_temp_file(
         return 0;
 }
 
-int run_editor(const EditFile *files) {
+static int run_editor(const EditFileContext *context) {
         int r;
 
-        assert(files);
+        assert(context);
 
         r = safe_fork("(editor)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_RLIMIT_NOFILE_SAFE|FORK_LOG|FORK_WAIT, NULL);
         if (r < 0)
@@ -179,8 +253,7 @@ int run_editor(const EditFile *files) {
                         argc = n_editor_args;
                 }
 
-                for (const EditFile *f = files; f->path; f++)
-                        argc += 2;
+                argc += context->n_files * 2;
 
                 args = newa(char*, argc + 1);
 
@@ -190,18 +263,18 @@ int run_editor(const EditFile *files) {
                                 args[i] = editor_args[i];
                 }
 
-                if (files[0].path && files[0].line > 1 && !files[1].path) {
+                if (context->n_files == 1 && context->files[0].line > 1) {
                         /* If editing a single file only, use the +LINE syntax to put cursor on the right line */
-                        if (asprintf(args + i, "+%u", files[0].line) < 0) {
+                        if (asprintf(args + i, "+%u", context->files[0].line) < 0) {
                                 (void) log_oom();
                                 _exit(EXIT_FAILURE);
                         }
 
                         i++;
-                        args[i++] = files[0].tmp;
+                        args[i++] = context->files[0].temp;
                 } else
-                        for (const EditFile *f = files; f->path; f++)
-                                args[i++] = f->tmp;
+                        FOREACH_ARRAY(f, context->files, context->n_files)
+                                args[i++] = f->temp;
 
                 args[i] = NULL;
 
@@ -226,7 +299,7 @@ int run_editor(const EditFile *files) {
         return 0;
 }
 
-int trim_edit_markers(const char *path, const char *marker_start, const char *marker_end) {
+static int trim_edit_markers(const char *path, const char *marker_start, const char *marker_end) {
         _cleanup_free_ char *old_contents = NULL, *new_contents = NULL;
         char *contents_start, *contents_end;
         const char *c = NULL;
@@ -266,3 +339,47 @@ int trim_edit_markers(const char *path, const char *marker_start, const char *ma
 
         return 1; /* Changed, but good */
 }
+
+int do_edit_files_and_install(EditFileContext *context) {
+        int r;
+
+        assert(context);
+
+        if (context->n_files == 0)
+                return log_debug_errno(SYNTHETIC_ERRNO(ENOENT), "Got no files to edit.");
+
+        FOREACH_ARRAY(i, context->files, context->n_files)
+                if (isempty(i->temp)) {
+                        r = create_edit_temp_file(i->path,
+                                                  i->original_path,
+                                                  i->comment_paths,
+                                                  context->marker_start,
+                                                  context->marker_end,
+                                                  &i->temp,
+                                                  &i->line);
+                        if (r < 0)
+                                return r;
+                }
+
+        r = run_editor(context);
+        if (r < 0)
+                return r;
+
+        FOREACH_ARRAY(i, context->files, context->n_files) {
+                /* Always call trim_edit_markers to tell if the temp file is empty */
+                r = trim_edit_markers(i->temp, context->marker_start, context->marker_end);
+                if (r < 0)
+                        return r;
+                if (r == 0) /* temp file doesn't carry actual changes, ignoring */
+                        continue;
+
+                r = RET_NERRNO(rename(i->temp, i->path));
+                if (r < 0)
+                        return log_error_errno(r, "Failed to rename \"%s\" to \"%s\": %m", i->temp, i->path);
+                i->temp = mfree(i->temp);
+
+                log_info("Successfully installed edited file '%s'.", i->path);
+        }
+
+        return 0;
+}
diff --git a/src/shared/edit-util.h b/src/shared/edit-util.h
index b20c09e896..7b705060e4 100644
--- a/src/shared/edit-util.h
+++ b/src/shared/edit-util.h
@@ -5,21 +5,28 @@
 
 typedef struct EditFile {
         char *path;
-        char *tmp;
+        char *original_path;
+        char **comment_paths;
+        char *temp;
         unsigned line;
 } EditFile;
 
-void edit_file_free_all(EditFile **ef);
+typedef struct EditFileContext {
+        EditFile *files;
+        size_t n_files;
+        const char *marker_start;
+        const char *marker_end;
+        bool remove_parent;
+} EditFileContext;
 
-int create_edit_temp_file(
-                const char *target_path,
-                const char *original_path,
-                char * const *comment_paths,
-                const char *marker_start,
-                const char *marker_end,
-                char **ret_temp_filename,
-                unsigned *ret_edit_line);
+void edit_file_context_done(EditFileContext *context);
+
+bool edit_files_contains(const EditFileContext *context, const char *path);
 
-int run_editor(const EditFile *files);
+int edit_files_add(
+                EditFileContext *context,
+                const char *path,
+                const char *original_path,
+                char * const *comment_paths);
 
-int trim_edit_markers(const char *path, const char *marker_start, const char *marker_end);
+int do_edit_files_and_install(EditFileContext *context);
diff --git a/src/systemctl/systemctl-edit.c b/src/systemctl/systemctl-edit.c
index 88ac98d8a2..b44e595b25 100644
--- a/src/systemctl/systemctl-edit.c
+++ b/src/systemctl/systemctl-edit.c
@@ -136,50 +136,43 @@ static int get_file_to_edit(
 }
 
 static int unit_file_create_new(
+                EditFileContext *context,
                 const LookupPaths *paths,
                 const char *unit_name,
                 const char *suffix,
-                char ** const original_unit_paths,
-                EditFile *ret_edit_file) {
+                char * const *original_unit_paths) {
 
-        _cleanup_free_ char *new_path = NULL, *tmp_path = NULL;
-        unsigned edit_line;
+        _cleanup_free_ char *new_path = NULL;
         const char *ending;
         int r;
 
+        assert(context);
         assert(unit_name);
-        assert(ret_edit_file);
 
         ending = strjoina(unit_name, suffix);
         r = get_file_to_edit(paths, ending, &new_path);
         if (r < 0)
                 return r;
 
-        r = create_edit_temp_file(new_path, NULL, original_unit_paths, EDIT_MARKER_START, EDIT_MARKER_END, &tmp_path, &edit_line);
+        r = edit_files_add(context, new_path, NULL, original_unit_paths);
         if (r < 0)
                 return r;
 
-        *ret_edit_file = (EditFile) {
-                .path = TAKE_PTR(new_path),
-                .tmp = TAKE_PTR(tmp_path),
-                .line = edit_line,
-        };
         return 0;
 }
 
 static int unit_file_create_copy(
+                EditFileContext *context,
                 const LookupPaths *paths,
                 const char *unit_name,
-                const char *fragment_path,
-                EditFile *ret_edit_file) {
+                const char *fragment_path) {
 
-        _cleanup_free_ char *new_path = NULL, *tmp_path = NULL;
-        unsigned edit_line;
+        _cleanup_free_ char *new_path = NULL;
         int r;
 
+        assert(context);
         assert(fragment_path);
         assert(unit_name);
-        assert(ret_edit_file);
 
         r = get_file_to_edit(paths, unit_name, &new_path);
         if (r < 0)
@@ -195,33 +188,27 @@ static int unit_file_create_copy(
                         return log_warning_errno(SYNTHETIC_ERRNO(EKEYREJECTED), "%s skipped.", unit_name);
         }
 
-        r = create_edit_temp_file(new_path, fragment_path, NULL, EDIT_MARKER_START, EDIT_MARKER_END, &tmp_path, &edit_line);
+        r = edit_files_add(context, new_path, fragment_path, NULL);
         if (r < 0)
                 return r;
 
-        *ret_edit_file = (EditFile) {
-                .path = TAKE_PTR(new_path),
-                .tmp = TAKE_PTR(tmp_path),
-                .line = edit_line,
-        };
         return 0;
 }
 
 static int find_paths_to_edit(
                 sd_bus *bus,
-                char **names,
-                EditFile **ret_edit_files) {
+                EditFileContext *context,
+                char **names) {
 
         _cleanup_(hashmap_freep) Hashmap *cached_name_map = NULL, *cached_id_map = NULL;
-        _cleanup_(edit_file_free_all) EditFile *edit_files = NULL;
         _cleanup_(lookup_paths_free) LookupPaths lp = {};
         _cleanup_free_ char *drop_in_alloc = NULL, *suffix = NULL;
         const char *drop_in;
-        size_t n_edit_files = 0;
         int r;
 
+        assert(bus);
+        assert(context);
         assert(names);
-        assert(ret_edit_files);
 
         if (isempty(arg_drop_in))
                 drop_in = "override.conf";
@@ -261,9 +248,6 @@ static int find_paths_to_edit(
                 if (r < 0)
                         return r;
 
-                if (!GREEDY_REALLOC0(edit_files, n_edit_files + 2))
-                        return log_oom();
-
                 if (!path) {
                         if (!arg_force) {
                                 log_info("Run 'systemctl edit%s --force --full %s' to create a new unit.",
@@ -275,11 +259,11 @@ static int find_paths_to_edit(
 
                         /* Create a new unit from scratch */
                         r = unit_file_create_new(
+                                        context,
                                         &lp,
                                         *name,
                                         arg_full ? NULL : suffix,
-                                        NULL,
-                                        edit_files + n_edit_files);
+                                        NULL);
                 } else {
                         _cleanup_free_ char *unit_name = NULL;
 
@@ -305,35 +289,36 @@ static int find_paths_to_edit(
 
                         if (arg_full)
                                 r = unit_file_create_copy(
+                                                context,
                                                 &lp,
                                                 unit_name,
-                                                path,
-                                                edit_files + n_edit_files);
+                                                path);
                         else {
                                 r = strv_prepend(&unit_paths, path);
                                 if (r < 0)
                                         return log_oom();
 
                                 r = unit_file_create_new(
+                                                context,
                                                 &lp,
                                                 unit_name,
                                                 suffix,
-                                                unit_paths,
-                                                edit_files + n_edit_files);
+                                                unit_paths);
                         }
                 }
                 if (r < 0)
                         return r;
-
-                n_edit_files++;
         }
 
-        *ret_edit_files = n_edit_files > 0 ? TAKE_PTR(edit_files) : NULL;
         return 0;
 }
 
 int verb_edit(int argc, char *argv[], void *userdata) {
-        _cleanup_(edit_file_free_all) EditFile *edit_files = NULL;
+        _cleanup_(edit_file_context_done) EditFileContext context = {
+                .marker_start = EDIT_MARKER_START,
+                .marker_end = EDIT_MARKER_END,
+                .remove_parent = !arg_full,
+        };
         _cleanup_(lookup_paths_free) LookupPaths lp = {};
         _cleanup_strv_free_ char **names = NULL;
         sd_bus *bus;
@@ -371,36 +356,13 @@ int verb_edit(int argc, char *argv[], void *userdata) {
                         return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Cannot edit %s: unit is masked.", *tmp);
         }
 
-        r = find_paths_to_edit(bus, names, &edit_files);
+        r = find_paths_to_edit(bus, &context, names);
         if (r < 0)
                 return r;
-        if (!edit_files)
-                return -ENOENT;
 
-        r = run_editor(edit_files);
+        r = do_edit_files_and_install(&context);
         if (r < 0)
-                goto end;
-
-        for (EditFile *f = edit_files; f->path; f++) {
-                /* If the temporary file is empty we ignore it. This allows the user to cancel the
-                 * modification. */
-                r = trim_edit_markers(f->tmp, EDIT_MARKER_START, EDIT_MARKER_END);
-                if (r < 0)
-                        goto end;
-                if (r == 0) /* has no actual contents? then ignore it */
-                        continue;
-
-                r = RET_NERRNO(rename(f->tmp, f->path));
-                if (r < 0) {
-                        log_error_errno(r, "Failed to rename \"%s\" to \"%s\": %m", f->tmp, f->path);
-                        goto end;
-                }
-
-                f->tmp = mfree(f->tmp);
-                log_info("Successfully installed edited file '%s'.", f->path);
-        }
-
-        r = 0;
+                return r;
 
         if (!arg_no_reload && !install_client_side()) {
                 r = daemon_reload(ACTION_RELOAD, /* graceful= */ false);
@@ -408,24 +370,5 @@ int verb_edit(int argc, char *argv[], void *userdata) {
                         r = 0;
         }
 
-end:
-        for (EditFile *f = ASSERT_PTR(edit_files); f->path; f++) {
-
-                if (f->tmp)
-                        (void) unlink(f->tmp);
-
-                /* Removing empty dropin dirs */
-                if (!arg_full) {
-                        _cleanup_free_ char *dir = NULL;
-
-                        r = path_extract_directory(f->path, &dir);
-                        if (r < 0)
-                                return log_error_errno(r, "Failed to extract directory from '%s': %m", f->path);
-
-                        /* No need to check if the dir is empty, rmdir does nothing if it is not the case. */
-                        (void) rmdir(dir);
-                }
-        }
-
         return r;
 }