Merge pull request #27174 from YHNdnzj/edit-util-fixlets

edit-util: several improvements

src/machine/machinectl.c

@@ -1454,9 +1454,7 @@ static int get_settings_path(const char *name, char **ret_path) {
 }
 
 static int edit_settings(int argc, char *argv[], void *userdata) {
-        _cleanup_(edit_file_context_done) EditFileContext context = {
-                .remove_parent = false,
-        };
+        _cleanup_(edit_file_context_done) EditFileContext context = {};
         int r;
 
         if (!on_tty())

src/shared/edit-util.c

@@ -12,11 +12,9 @@
 #include "mkdir-label.h"
 #include "path-util.h"
 #include "process-util.h"
-#include "selinux-util.h"
-#include "stat-util.h"
 #include "string-util.h"
 #include "strv.h"
-#include "tmpfile-util.h"
+#include "tmpfile-util-label.h"
 
 void edit_file_context_done(EditFileContext *context) {
         int r;
@@ -103,6 +101,9 @@ int edit_files_add(
 
 static int create_edit_temp_file(EditFile *e) {
         _cleanup_(unlink_and_freep) char *temp = NULL;
+        _cleanup_fclose_ FILE *f = NULL;
+        const char *source;
+        bool has_original, has_target;
         unsigned line = 1;
         int r;
 
@@ -114,62 +115,37 @@ static int create_edit_temp_file(EditFile *e) {
         if (e->temp)
                 return 0;
 
-        r = tempfn_random(e->path, NULL, &temp);
-        if (r < 0)
-                return log_error_errno(r, "Failed to determine temporary filename for \"%s\": %m", e->path);
-
         r = mkdir_parents_label(e->path, 0755);
         if (r < 0)
-                return log_error_errno(r, "Failed to create parent directories for \"%s\": %m", e->path);
+                return log_error_errno(r, "Failed to create parent directories for '%s': %m", e->path);
 
-        if (!e->original_path && !e->comment_paths) {
-                r = mac_selinux_create_file_prepare(e->path, S_IFREG);
-                if (r < 0)
-                        return r;
+        r = fopen_temporary_label(e->path, e->path, &f, &temp);
+        if (r < 0)
+                return log_error_errno(r, "Failed to create temporary file for '%s': %m", e->path);
 
-                r = touch(temp);
-                mac_selinux_create_file_clear();
-                if (r < 0)
-                        return log_error_errno(r, "Failed to create temporary file \"%s\": %m", temp);
-        }
+        if (fchmod(fileno(f), 0644) < 0)
+                return log_error_errno(errno, "Failed to change mode of temporary file '%s': %m", temp);
 
-        if (e->original_path) {
-                r = mac_selinux_create_file_prepare(e->path, S_IFREG);
-                if (r < 0)
-                        return r;
+        has_original = e->original_path && access(e->original_path, F_OK) >= 0;
+        has_target = access(e->path, F_OK) >= 0;
 
-                r = copy_file(e->original_path, temp, 0, 0644, COPY_REFLINK);
-                if (r == -ENOENT) {
-                        r = touch(temp);
-                        mac_selinux_create_file_clear();
-                        if (r < 0)
-                                return log_error_errno(r, "Failed to create temporary file \"%s\": %m", temp);
-                } else {
-                        mac_selinux_create_file_clear();
-                        if (r < 0)
-                                return log_error_errno(r, "Failed to create temporary file for \"%s\": %m", e->path);
-                }
-        }
+        if (has_original && (!has_target || e->context->overwrite_with_origin))
+                /* We are asked to overwrite target with original_path or target doesn't exist. */
+                source = e->original_path;
+        else if (has_target)
+                /* Target exists and shouldn't be overwritten. */
+                source = e->path;
+        else
+                source = NULL;
 
         if (e->comment_paths) {
-                _cleanup_free_ char *target_contents = NULL;
-                _cleanup_fclose_ FILE *f = NULL;
+                _cleanup_free_ char *source_contents = NULL;
 
-                r = mac_selinux_create_file_prepare(e->path, S_IFREG);
-                if (r < 0)
-                        return r;
-
-                f = fopen(temp, "we");
-                mac_selinux_create_file_clear();
-                if (!f)
-                        return log_error_errno(errno, "Failed to open temporary file \"%s\": %m", temp);
-
-                if (fchmod(fileno(f), 0644) < 0)
-                        return log_error_errno(errno, "Failed to change mode of temporary file \"%s\": %m", temp);
-
-                r = read_full_file(e->path, &target_contents, NULL);
-                if (r < 0 && r != -ENOENT)
-                        return log_error_errno(r, "Failed to read target file \"%s\": %m", e->path);
+                if (source) {
+                        r = read_full_file(source, &source_contents, NULL);
+                        if (r < 0)
+                                return log_error_errno(r, "Failed to read source file '%s': %m", source);
+                }
 
                 fprintf(f,
                         "### Editing %s\n"
@@ -180,41 +156,47 @@ static int create_edit_temp_file(EditFile *e) {
                         "%s\n",
                         e->path,
                         e->context->marker_start,
-                        strempty(target_contents),
-                        target_contents && endswith(target_contents, "\n") ? "" : "\n",
+                        strempty(source_contents),
+                        source_contents && endswith(source_contents, "\n") ? "" : "\n",
                         e->context->marker_end);
 
                 line = 4; /* Start editing at the contents area */
 
-                /* Add a comment with the contents of the original files */
                 STRV_FOREACH(path, e->comment_paths) {
-                        _cleanup_free_ char *contents = NULL;
+                        _cleanup_free_ char *comment = NULL;
 
-                        /* Skip the file that's being edited, already processed in above */
-                        if (path_equal(*path, e->path))
+                        /* Skip the file which is being edited and the source file (can be the same) */
+                        if (PATH_IN_SET(*path, e->path, source))
                                 continue;
 
-                        r = read_full_file(*path, &contents, NULL);
+                        r = read_full_file(*path, &comment, NULL);
                         if (r < 0)
-                                return log_error_errno(r, "Failed to read original file \"%s\": %m", *path);
+                                return log_error_errno(r, "Failed to read comment file '%s': %m", *path);
 
                         fprintf(f, "\n\n### %s", *path);
-                        if (!isempty(contents)) {
-                                _cleanup_free_ char *commented_contents = NULL;
 
-                                commented_contents = strreplace(strstrip(contents), "\n", "\n# ");
-                                if (!commented_contents)
+                        if (!isempty(comment)) {
+                                _cleanup_free_ char *c = NULL;
+
+                                c = strreplace(strstrip(comment), "\n", "\n# ");
+                                if (!c)
                                         return log_oom();
 
-                                fprintf(f, "\n# %s", commented_contents);
+                                fprintf(f, "\n# %s", c);
                         }
                 }
-
-                r = fflush_and_check(f);
-                if (r < 0)
-                        return log_error_errno(r, "Failed to create temporary file \"%s\": %m", temp);
+        } else if (source) {
+                r = copy_file_fd(source, fileno(f), COPY_REFLINK);
+                if (r < 0) {
+                        assert(r != -ENOENT);
+                        return log_error_errno(r, "Failed to copy file '%s' to temporary file '%s': %m", source, temp);
+                }
         }
 
+        r = fflush_and_check(f);
+        if (r < 0)
+                return log_error_errno(r, "Failed to write to temporary file '%s': %m", temp);
+
         e->temp = TAKE_PTR(temp);
         e->line = line;
 
@@ -310,7 +292,7 @@ static int strip_edit_temp_file(EditFile *e) {
 
         r = read_full_file(e->temp, &old_contents, NULL);
         if (r < 0)
-                return log_error_errno(r, "Failed to read temporary file \"%s\": %m", e->temp);
+                return log_error_errno(r, "Failed to read temporary file '%s': %m", e->temp);
 
         if (e->context->marker_start) {
                 /* Trim out the lines between the two markers */
@@ -344,7 +326,7 @@ static int strip_edit_temp_file(EditFile *e) {
 
         r = write_string_file(e->temp, new_contents, WRITE_STRING_FILE_CREATE | WRITE_STRING_FILE_TRUNCATE | WRITE_STRING_FILE_AVOID_NEWLINE);
         if (r < 0)
-                return log_error_errno(r, "Failed to modify temporary file \"%s\": %m", e->temp);
+                return log_error_errno(r, "Failed to strip temporary file '%s': %m", e->temp);
 
         return 1; /* Contents have real changes and are changed after stripping */
 }
@@ -377,7 +359,10 @@ int do_edit_files_and_install(EditFileContext *context) {
 
                 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);
+                        return log_error_errno(r,
+                                               "Failed to rename temporary file '%s' to target file '%s': %m",
+                                               i->temp,
+                                               i->path);
                 i->temp = mfree(i->temp);
 
                 log_info("Successfully installed edited file '%s'.", i->path);

src/shared/edit-util.h

@@ -3,6 +3,9 @@
 
 #include <stdbool.h>
 
+#define DROPIN_MARKER_START "### Anything between here and the comment below will become the contents of the drop-in file"
+#define DROPIN_MARKER_END "### Edits below this comment will be discarded"
+
 typedef struct EditFile EditFile;
 typedef struct EditFileContext EditFileContext;
 
@@ -21,6 +24,7 @@ struct EditFileContext {
         const char *marker_start;
         const char *marker_end;
         bool remove_parent;
+        bool overwrite_with_origin; /* whether to always overwrite target with original file */
 };
 
 void edit_file_context_done(EditFileContext *context);

src/systemctl/systemctl-edit.c

@@ -13,9 +13,6 @@
 #include "systemctl.h"
 #include "terminal-util.h"
 
-#define EDIT_MARKER_START "### Anything between here and the comment below will become the contents of the drop-in file"
-#define EDIT_MARKER_END "### Edits below this comment will be discarded"
-
 int verb_cat(int argc, char *argv[], void *userdata) {
         _cleanup_(hashmap_freep) Hashmap *cached_name_map = NULL, *cached_id_map = NULL;
         _cleanup_(lookup_paths_free) LookupPaths lp = {};
@@ -316,9 +313,10 @@ static int find_paths_to_edit(
 
 int verb_edit(int argc, char *argv[], void *userdata) {
         _cleanup_(edit_file_context_done) EditFileContext context = {
-                .marker_start = EDIT_MARKER_START,
-                .marker_end = EDIT_MARKER_END,
+                .marker_start = DROPIN_MARKER_START,
+                .marker_end = DROPIN_MARKER_END,
                 .remove_parent = !arg_full,
+                .overwrite_with_origin = true,
         };
         _cleanup_(lookup_paths_free) LookupPaths lp = {};
         _cleanup_strv_free_ char **names = NULL;