From: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Date: Sat, 2 Dec 2023 15:41:46 +0000 (+0100)
Subject: systemctl: add message when edit is aborted
X-Git-Tag: v256-rc1~1566^2~2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=232f017b1a879c4d099dcbb644f60e44ae2b8dbf;p=thirdparty%2Fsystemd.git

systemctl: add message when edit is aborted

If the user edits result in an empty file (after stripping), we would silently
not do anything, aborting the edit. This is rather confusing, let's emit a
notice:

  $ build/systemctl --user edit asdf.service --full --stdin </dev/null
  /home/zbyszek/.config/systemd/user/asdf.service: after editing, new contents are empty, not writing file.

(This also works with an editor, instead of --stdin. The message is printed on
the console and is visible after the editor exits.)

While at it, fix the condition to skip writing file after stripping. We had
"old_contents", but we modified that string. In some code flows, we would
compare the stripped old contents (i.e. a string which by defintion doesn't
have a newline at the end) with a string to which we just appended a newline
(i.e. a string which by defintion has a newline at the end).
---

diff --git a/src/shared/edit-util.c b/src/shared/edit-util.c
index 1c8f0917173..dd77b0e20e3 100644
--- a/src/shared/edit-util.c
+++ b/src/shared/edit-util.c
@@ -294,7 +294,7 @@ static int run_editor(const EditFileContext *context) {
 }
 
 static int strip_edit_temp_file(EditFile *e) {
-        _cleanup_free_ char *old_contents = NULL, *new_contents = NULL;
+        _cleanup_free_ char *old_contents = NULL, *tmp = NULL, *new_contents = NULL;
         const char *stripped;
         int r;
 
@@ -306,15 +306,17 @@ static int strip_edit_temp_file(EditFile *e) {
         if (r < 0)
                 return log_error_errno(r, "Failed to read temporary file '%s': %m", e->temp);
 
+        tmp = strdup(old_contents);
+        if (!tmp)
+                return log_oom();
+
         if (e->context->marker_start) {
                 /* Trim out the lines between the two markers */
                 char *contents_start, *contents_end;
 
                 assert(e->context->marker_end);
 
-                contents_start = strstrafter(old_contents, e->context->marker_start);
-                if (!contents_start)
-                        contents_start = old_contents;
+                contents_start = strstrafter(tmp, e->context->marker_start) ?: tmp;
 
                 contents_end = strstr(contents_start, e->context->marker_end);
                 if (contents_end)
@@ -322,9 +324,13 @@ static int strip_edit_temp_file(EditFile *e) {
 
                 stripped = strstrip(contents_start);
         } else
-                stripped = strstrip(old_contents);
-        if (isempty(stripped))
-                return 0; /* File is empty (has no real changes) */
+                stripped = strstrip(tmp);
+
+        if (isempty(stripped)) {
+                /* File is empty (has no real changes) */
+                log_notice("%s: after editing, new contents are empty, not writing file.", e->path);
+                return 0;
+        }
 
         /* Trim prefix and suffix, but ensure suffixed by single newline */
         new_contents = strjoin(stripped, "\n");
@@ -332,13 +338,14 @@ static int strip_edit_temp_file(EditFile *e) {
                 return log_oom();
 
         if (streq(old_contents, new_contents)) /* Don't touch the file if the above didn't change a thing */
-                return 1; /* Contents unchanged after stripping but has changes */
+                return 1; /* Contents have real changes */
 
-        r = write_string_file(e->temp, new_contents, WRITE_STRING_FILE_CREATE | WRITE_STRING_FILE_TRUNCATE | WRITE_STRING_FILE_AVOID_NEWLINE);
+        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 strip temporary file '%s': %m", e->temp);
 
-        return 1; /* Contents have real changes and are changed after stripping */
+        return 1; /* Contents have real changes */
 }
 
 int do_edit_files_and_install(EditFileContext *context) {