From 6e5d0e319ee45c6c2588a53468e98bd1b9c62f0d Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 5 Apr 2023 16:48:27 +0800 Subject: [PATCH 1/4] edit-util: add DROPIN_MARKER_{START,END} --- src/shared/edit-util.h | 3 +++ src/systemctl/systemctl-edit.c | 7 ++----- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/shared/edit-util.h b/src/shared/edit-util.h index 63c6190ef8..e57ca80a49 100644 --- a/src/shared/edit-util.h +++ b/src/shared/edit-util.h @@ -3,6 +3,9 @@ #include +#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; diff --git a/src/systemctl/systemctl-edit.c b/src/systemctl/systemctl-edit.c index 5f42dc239f..ff16b73229 100644 --- a/src/systemctl/systemctl-edit.c +++ b/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,8 +313,8 @@ 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, }; _cleanup_(lookup_paths_free) LookupPaths lp = {}; From 54ad6aa164042a7e4c49abf59ad1d728c1bb6d0b Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Fri, 7 Apr 2023 01:41:24 +0800 Subject: [PATCH 2/4] edit-util: make original_path and comment_paths work together --- src/shared/edit-util.c | 114 +++++++++++++++++------------------------ 1 file changed, 47 insertions(+), 67 deletions(-) diff --git a/src/shared/edit-util.c b/src/shared/edit-util.c index f1eb7987f4..097ef6a791 100644 --- a/src/shared/edit-util.c +++ b/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,35 @@ 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) + source = e->original_path; + else if (has_target) + 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 +154,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; From bc6c7a58c25f9e974996d43473fbfb1273d746c0 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Fri, 7 Apr 2023 01:41:24 +0800 Subject: [PATCH 3/4] edit-util: introduce overwrite_with_origin switch Before this commit, if `original_path` is given, it will always be used to overwrite `path`. After this commit, it's controlled by the newly-added switch `overwrite_with_origin`. --- src/machine/machinectl.c | 4 +--- src/shared/edit-util.c | 4 +++- src/shared/edit-util.h | 1 + src/systemctl/systemctl-edit.c | 1 + 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/machine/machinectl.c b/src/machine/machinectl.c index 6a29a32bcd..7eb2235667 100644 --- a/src/machine/machinectl.c +++ b/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()) diff --git a/src/shared/edit-util.c b/src/shared/edit-util.c index 097ef6a791..f482c2ac8b 100644 --- a/src/shared/edit-util.c +++ b/src/shared/edit-util.c @@ -129,9 +129,11 @@ static int create_edit_temp_file(EditFile *e) { has_original = e->original_path && access(e->original_path, F_OK) >= 0; has_target = access(e->path, F_OK) >= 0; - if (has_original) + 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; diff --git a/src/shared/edit-util.h b/src/shared/edit-util.h index e57ca80a49..83b3df8683 100644 --- a/src/shared/edit-util.h +++ b/src/shared/edit-util.h @@ -24,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); diff --git a/src/systemctl/systemctl-edit.c b/src/systemctl/systemctl-edit.c index ff16b73229..561b01a67a 100644 --- a/src/systemctl/systemctl-edit.c +++ b/src/systemctl/systemctl-edit.c @@ -316,6 +316,7 @@ int verb_edit(int argc, char *argv[], void *userdata) { .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; From 77c9bb177aaed9395770c5bacf790b4c41bb3999 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Fri, 7 Apr 2023 03:55:39 +0800 Subject: [PATCH 4/4] edit-util: improve log messages --- src/shared/edit-util.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/shared/edit-util.c b/src/shared/edit-util.c index f482c2ac8b..9fd74973aa 100644 --- a/src/shared/edit-util.c +++ b/src/shared/edit-util.c @@ -292,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 */ @@ -326,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 */ } @@ -359,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);