From 82443be506be8b421fdd2771ad3e9a7d3014cb7b Mon Sep 17 00:00:00 2001 From: williamvds Date: Wed, 21 Oct 2020 17:14:37 +0100 Subject: [PATCH 1/3] Add strv_prepend Inserts a copy of the value at the head of the list. --- src/basic/strv.c | 13 +++++++++++++ src/basic/strv.h | 1 + 2 files changed, 14 insertions(+) diff --git a/src/basic/strv.c b/src/basic/strv.c index b2b6de388a..c5e6dd5f21 100644 --- a/src/basic/strv.c +++ b/src/basic/strv.c @@ -537,6 +537,19 @@ int strv_consume_prepend(char ***l, char *value) { return r; } +int strv_prepend(char ***l, const char *value) { + char *v; + + if (!value) + return 0; + + v = strdup(value); + if (!v) + return -ENOMEM; + + return strv_consume_prepend(l, v); +} + int strv_extend(char ***l, const char *value) { char *v; diff --git a/src/basic/strv.h b/src/basic/strv.h index 919fabf75a..f2aa6c239b 100644 --- a/src/basic/strv.h +++ b/src/basic/strv.h @@ -34,6 +34,7 @@ size_t strv_length(char * const *l) _pure_; int strv_extend_strv(char ***a, char * const *b, bool filter_duplicates); int strv_extend_strv_concat(char ***a, char * const *b, const char *suffix); +int strv_prepend(char ***l, const char *value); int strv_extend(char ***l, const char *value); int strv_extendf(char ***l, const char *format, ...) _printf_(2,0); int strv_extend_front(char ***l, const char *value); From 6797a74f789210dc0ea475a1d066688c6ce5e929 Mon Sep 17 00:00:00 2001 From: williamvds Date: Wed, 21 Oct 2020 17:18:25 +0100 Subject: [PATCH 2/3] Add WRITE_STRING_FILE_TRUNCATE to set O_TRUNC --- src/basic/fileio.c | 3 ++- src/basic/fileio.h | 17 +++++++++-------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/basic/fileio.c b/src/basic/fileio.c index c5a093a857..050c8709f8 100644 --- a/src/basic/fileio.c +++ b/src/basic/fileio.c @@ -252,7 +252,8 @@ int write_string_file_ts( /* We manually build our own version of fopen(..., "we") that works without O_CREAT and with O_NOFOLLOW if needed. */ fd = open(fn, O_WRONLY|O_CLOEXEC|O_NOCTTY | (FLAGS_SET(flags, WRITE_STRING_FILE_NOFOLLOW) ? O_NOFOLLOW : 0) | - (FLAGS_SET(flags, WRITE_STRING_FILE_CREATE) ? O_CREAT : 0), + (FLAGS_SET(flags, WRITE_STRING_FILE_CREATE) ? O_CREAT : 0) | + (FLAGS_SET(flags, WRITE_STRING_FILE_TRUNCATE) ? O_TRUNC : 0), (FLAGS_SET(flags, WRITE_STRING_FILE_MODE_0600) ? 0600 : 0666)); if (fd < 0) { r = -errno; diff --git a/src/basic/fileio.h b/src/basic/fileio.h index 9cba5a90e3..963d7d08fc 100644 --- a/src/basic/fileio.h +++ b/src/basic/fileio.h @@ -16,14 +16,15 @@ typedef enum { WRITE_STRING_FILE_CREATE = 1 << 0, - WRITE_STRING_FILE_ATOMIC = 1 << 1, - WRITE_STRING_FILE_AVOID_NEWLINE = 1 << 2, - WRITE_STRING_FILE_VERIFY_ON_FAILURE = 1 << 3, - WRITE_STRING_FILE_SYNC = 1 << 4, - WRITE_STRING_FILE_DISABLE_BUFFER = 1 << 5, - WRITE_STRING_FILE_NOFOLLOW = 1 << 6, - WRITE_STRING_FILE_MKDIR_0755 = 1 << 7, - WRITE_STRING_FILE_MODE_0600 = 1 << 8, + WRITE_STRING_FILE_TRUNCATE = 1 << 1, + WRITE_STRING_FILE_ATOMIC = 1 << 2, + WRITE_STRING_FILE_AVOID_NEWLINE = 1 << 3, + WRITE_STRING_FILE_VERIFY_ON_FAILURE = 1 << 4, + WRITE_STRING_FILE_SYNC = 1 << 5, + WRITE_STRING_FILE_DISABLE_BUFFER = 1 << 6, + WRITE_STRING_FILE_NOFOLLOW = 1 << 7, + WRITE_STRING_FILE_MKDIR_0755 = 1 << 8, + WRITE_STRING_FILE_MODE_0600 = 1 << 9, /* And before you wonder, why write_string_file_atomic_label_ts() is a separate function instead of just one more flag here: it's about linking: we don't want to pull -lselinux into all users of write_string_file() From 85c5d313b5c92115f5c77663e736bcf21e99f02f Mon Sep 17 00:00:00 2001 From: williamvds Date: Wed, 21 Oct 2020 17:19:05 +0100 Subject: [PATCH 3/3] systemctl: show original contents when editing unit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A comment indicates the start of the new contents of the override file, and another indicates that lines following it will be discarded once editing is finished. The contents of the unit file and drop-ins are listed out after this last marker. Adds WRITE_STRING_FILE_TRUNCATE to set O_TRUNC when opening a file. Thanks to cgzones for providing the required SELinux function calls. Co-authored-by: Christian Göttsche --- TODO | 3 - src/systemctl/systemctl-edit.c | 142 ++++++++++++++++++++++++++++----- 2 files changed, 122 insertions(+), 23 deletions(-) diff --git a/TODO b/TODO index b509ee2594..4c10401e4f 100644 --- a/TODO +++ b/TODO @@ -813,9 +813,6 @@ Features: * systemctl: if some operation fails, show log output? -* systemctl edit: use equivalent of cat() to insert existing config as a comment, prepended with #. - Upon editor exit, lines with one # are removed, lines with two # are left with one #, etc. - * exponential backoff in timesyncd when we cannot reach a server * timesyncd: add ugly bus calls to set NTP servers per-interface, for usage by NM diff --git a/src/systemctl/systemctl-edit.c b/src/systemctl/systemctl-edit.c index f4b256335a..d6f595d477 100644 --- a/src/systemctl/systemctl-edit.c +++ b/src/systemctl/systemctl-edit.c @@ -2,6 +2,8 @@ #include "bus-error.h" #include "copy.h" +#include "fd-util.h" +#include "fileio.h" #include "fs-util.h" #include "mkdir.h" #include "pager.h" @@ -17,6 +19,9 @@ #include "terminal-util.h" #include "tmpfile-util.h" +#define EDIT_MARKER_START "### Anything between here and the comment below will become the new contents of the file" +#define EDIT_MARKER_END "### Lines below this comment will be discarded" + int 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 = {}; @@ -106,12 +111,11 @@ int cat(int argc, char *argv[], void *userdata) { return rc; } -static int create_edit_temp_file(const char *new_path, const char *original_path, char **ret_tmp_fn) { +static int create_edit_temp_file(const char *new_path, const char *original_path, char ** const original_unit_paths, char **ret_tmp_fn) { _cleanup_free_ char *t = NULL; int r; assert(new_path); - assert(original_path); assert(ret_tmp_fn); r = tempfn_random(new_path, NULL, &t); @@ -122,25 +126,78 @@ static int create_edit_temp_file(const char *new_path, const char *original_path if (r < 0) return log_error_errno(r, "Failed to create directories for \"%s\": %m", new_path); - r = mac_selinux_create_file_prepare(original_path, S_IFREG); - if (r < 0) - return r; + if (original_path) { + r = mac_selinux_create_file_prepare(new_path, S_IFREG); + if (r < 0) + return r; - r = copy_file(original_path, t, 0, 0644, 0, 0, COPY_REFLINK); - if (r == -ENOENT) { + r = copy_file(original_path, t, 0, 0644, 0, 0, COPY_REFLINK); + if (r == -ENOENT) { + r = touch(t); + mac_selinux_create_file_clear(); + if (r < 0) + return log_error_errno(r, "Failed to create temporary file \"%s\": %m", t); + } else { + mac_selinux_create_file_clear(); + if (r < 0) + return log_error_errno(r, "Failed to create temporary file for \"%s\": %m", new_path); + } + } else if (original_unit_paths) { + _cleanup_free_ char *new_contents = NULL; + _cleanup_fclose_ FILE *f = NULL; + char **path; + size_t size; - r = touch(t); + r = mac_selinux_create_file_prepare(new_path, S_IFREG); + if (r < 0) + return r; + f = fopen(t, "we"); mac_selinux_create_file_clear(); + if (!f) + return log_error_errno(errno, "Failed to open \"%s\": %m", t); + r = fchmod(fileno(f), 0644); + if (r < 0) + return log_error_errno(errno, "Failed to change mode of \"%s\": %m", t); + + r = read_full_file(new_path, &new_contents, &size); + if (r < 0 && r != -ENOENT) + return log_error_errno(r, "Failed to read \"%s\": %m", new_path); + + fprintf(f, + "### Editing %s\n" + EDIT_MARKER_START + "\n\n%s%s\n" + EDIT_MARKER_END, + new_path, + strempty(new_contents), + new_contents && endswith(new_contents, "\n") ? "" : "\n"); + + /* Add a comment with the contents of the original unit files */ + STRV_FOREACH(path, original_unit_paths) { + _cleanup_free_ char *contents = NULL; + + /* Skip the file that's being edited */ + if (path_equal(*path, new_path)) + continue; + + r = read_full_file(*path, &contents, &size); + if (r < 0) + return log_error_errno(r, "Failed to read \"%s\": %m", *path); + + fprintf(f, "\n\n### %s", *path); + if (!isempty(contents)) { + contents = strreplace(strstrip(contents), "\n", "\n# "); + if (!contents) + return log_oom(); + fprintf(f, "\n# %s", contents); + } + } + + r = fflush_and_check(f); if (r < 0) return log_error_errno(r, "Failed to create temporary file \"%s\": %m", t); - - } else { - mac_selinux_create_file_clear(); - - if (r < 0) - return log_error_errno(r, "Failed to create temporary file for \"%s\": %m", new_path); } *ret_tmp_fn = TAKE_PTR(t); @@ -185,6 +242,7 @@ static int unit_file_create_new( const LookupPaths *paths, const char *unit_name, const char *suffix, + char ** const original_unit_paths, char **ret_new_path, char **ret_tmp_path) { @@ -201,7 +259,7 @@ static int unit_file_create_new( if (r < 0) return r; - r = create_edit_temp_file(new_path, new_path, &tmp_path); + r = create_edit_temp_file(new_path, NULL, original_unit_paths, &tmp_path); if (r < 0) return r; @@ -240,7 +298,7 @@ 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, &tmp_path); + r = create_edit_temp_file(new_path, fragment_path, NULL, &tmp_path); if (r < 0) return r; @@ -332,9 +390,10 @@ static int find_paths_to_edit(sd_bus *bus, char **names, char ***paths) { STRV_FOREACH(name, names) { _cleanup_free_ char *path = NULL, *new_path = NULL, *tmp_path = NULL, *tmp_name = NULL; + _cleanup_strv_free_ char **unit_paths = NULL; const char *unit_name; - r = unit_find_paths(bus, *name, &lp, false, &cached_name_map, &cached_id_map, &path, NULL); + r = unit_find_paths(bus, *name, &lp, false, &cached_name_map, &cached_id_map, &path, &unit_paths); if (r == -EKEYREJECTED) { /* If loading of the unit failed server side complete, then the server won't tell us * the unit file path. In that case, find the file client side. */ @@ -361,7 +420,7 @@ static int find_paths_to_edit(sd_bus *bus, char **names, char ***paths) { unit_name = *name; r = unit_file_create_new(&lp, unit_name, arg_full ? NULL : ".d/override.conf", - &new_path, &tmp_path); + NULL, &new_path, &tmp_path); } else { assert(path); @@ -384,8 +443,13 @@ static int find_paths_to_edit(sd_bus *bus, char **names, char ***paths) { if (arg_full) r = unit_file_create_copy(&lp, unit_name, path, &new_path, &tmp_path); - else - r = unit_file_create_new(&lp, unit_name, ".d/override.conf", &new_path, &tmp_path); + else { + r = strv_prepend(&unit_paths, path); + if (r < 0) + return log_oom(); + + r = unit_file_create_new(&lp, unit_name, ".d/override.conf", unit_paths, &new_path, &tmp_path); + } } if (r < 0) return r; @@ -400,6 +464,40 @@ static int find_paths_to_edit(sd_bus *bus, char **names, char ***paths) { return 0; } +static int trim_edit_markers(const char *path) { + _cleanup_free_ char *contents = NULL; + char *contents_start = NULL; + const char *contents_end = NULL; + size_t size; + int r; + + /* Trim out the lines between the two markers */ + r = read_full_file(path, &contents, &size); + if (r < 0) + return log_error_errno(r, "Failed to read temporary file \"%s\": %m", path); + + contents_start = strstr(contents, EDIT_MARKER_START); + if (contents_start) + contents_start += strlen(EDIT_MARKER_START); + else + contents_start = contents; + + contents_end = strstr(contents_start, EDIT_MARKER_END); + if (contents_end) + strshorten(contents_start, contents_end - contents_start); + + contents_start = strstrip(contents_start); + + /* Write new contents if the trimming actually changed anything */ + if (strlen(contents) != size) { + r = write_string_file(path, contents_start, 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", path); + } + + return 0; +} + int edit(int argc, char *argv[], void *userdata) { _cleanup_(lookup_paths_free) LookupPaths lp = {}; _cleanup_strv_free_ char **names = NULL; @@ -452,6 +550,10 @@ int edit(int argc, char *argv[], void *userdata) { STRV_FOREACH_PAIR(original, tmp, paths) { /* If the temporary file is empty we ignore it. This allows the user to cancel the * modification. */ + r = trim_edit_markers(*tmp); + if (r < 0) + continue; + if (null_or_empty_path(*tmp)) { log_warning("Editing \"%s\" canceled: temporary file is empty.", *original); continue;