]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
edit-util: several cleanups for --stdin handling
authorMike Yuan <me@yhndnzj.com>
Sun, 11 Aug 2024 13:41:07 +0000 (15:41 +0200)
committerMike Yuan <me@yhndnzj.com>
Mon, 12 Aug 2024 14:23:23 +0000 (16:23 +0200)
Follow-up for 329050c5e2c7e9561699f87b5edb72edd0d54c96

I don't particularly favor the duplicated strstrip()
and such, so let's ensure if we get fixed data it's
only trimmed once. Subsequently we can benefit more
by making all copies reflinks.

src/shared/edit-util.c

index b1f2aade2753c628f5a7b6216537c6f85692b160..5dd8595a36937c0afcff1f225e05137fa8293622 100644 (file)
@@ -111,6 +111,9 @@ int edit_files_add(
 
 static int populate_edit_temp_file(EditFile *e, FILE *f, const char *filename) {
         assert(e);
+        assert(e->context);
+        assert(!e->context->stdin);
+        assert(e->path);
         assert(f);
         assert(filename);
 
@@ -197,6 +200,7 @@ static int create_edit_temp_file(EditFile *e, const char *contents, size_t conte
         assert(e->path);
         assert(!e->comment_paths || (e->context->marker_start && e->context->marker_end));
         assert(contents || contents_size == 0);
+        assert(e->context->stdin == !!contents);
 
         if (e->temp)
                 return 0;
@@ -215,7 +219,7 @@ static int create_edit_temp_file(EditFile *e, const char *contents, size_t conte
         if (e->context->stdin) {
                 if (fwrite(contents, 1, contents_size, f) != contents_size)
                         return log_error_errno(SYNTHETIC_ERRNO(EIO),
-                                               "Failed to copy input to temporary file '%s'.", temp);
+                                               "Failed to write stdin data to temporary file '%s'.", temp);
         } else {
                 r = populate_edit_temp_file(e, f, temp);
                 if (r < 0)
@@ -341,11 +345,8 @@ static int strip_edit_temp_file(EditFile *e) {
         } else
                 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;
-        }
+        if (isempty(stripped))
+                return 0; /* File is empty (has no real changes) */
 
         /* Trim prefix and suffix, but ensure suffixed by single newline */
         new_contents = strjoin(stripped, "\n");
@@ -363,9 +364,70 @@ static int strip_edit_temp_file(EditFile *e) {
         return 1; /* Contents have real changes */
 }
 
+static int edit_file_install_one(EditFile *e) {
+        int r;
+
+        assert(e);
+        assert(e->path);
+        assert(e->temp);
+
+        r = strip_edit_temp_file(e);
+        if (r <= 0)
+                return r;
+
+        r = RET_NERRNO(rename(e->temp, e->path));
+        if (r < 0)
+                return log_error_errno(r,
+                                       "Failed to rename temporary file '%s' to target file '%s': %m",
+                                       e->temp, e->path);
+        e->temp = mfree(e->temp);
+
+        return 1;
+}
+
+static int edit_file_install_one_stdin(EditFile *e, const char *contents, size_t contents_size, int *fd) {
+        int r;
+
+        assert(e);
+        assert(e->path);
+        assert(contents || contents_size == 0);
+        assert(fd);
+
+        if (contents_size == 0)
+                return 0;
+
+        if (*fd >= 0) {
+                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);
+
+                r = copy_file_atomic_at(*fd, NULL, AT_FDCWD, e->path, 0644, COPY_REFLINK|COPY_REPLACE|COPY_MAC_CREATE);
+                if (r < 0)
+                        return log_error_errno(r, "Failed to copy stdin contents to '%s': %m", e->path);
+
+                return 1;
+        }
+
+        r = create_edit_temp_file(e, contents, contents_size);
+        if (r < 0)
+                return r;
+
+        _cleanup_close_ int tfd = open(e->temp, O_PATH|O_CLOEXEC);
+        if (tfd < 0)
+                return log_error_errno(errno, "Failed to pin temporary file '%s': %m", e->temp);
+
+        r = edit_file_install_one(e);
+        if (r <= 0)
+                return r;
+
+        *fd = TAKE_FD(tfd);
+
+        return 1;
+}
+
 int do_edit_files_and_install(EditFileContext *context) {
-        _cleanup_free_ char *data = NULL;
-        size_t data_size = 0;
+        _cleanup_free_ char *stdin_data = NULL;
+        size_t stdin_size = 0;
         int r;
 
         assert(context);
@@ -374,38 +436,40 @@ int do_edit_files_and_install(EditFileContext *context) {
                 return log_debug_errno(SYNTHETIC_ERRNO(ENOENT), "Got no files to edit.");
 
         if (context->stdin) {
-                r = read_full_stream(stdin, &data, &data_size);
+                r = read_full_stream(stdin, &stdin_data, &stdin_size);
                 if (r < 0)
                         return log_error_errno(r, "Failed to read stdin: %m");
-        }
-
-        FOREACH_ARRAY(editfile, context->files, context->n_files) {
-                r = create_edit_temp_file(editfile, data, data_size);
-                if (r < 0)
-                        return r;
-        }
+        } else {
+                FOREACH_ARRAY(editfile, context->files, context->n_files) {
+                        r = create_edit_temp_file(editfile, /* contents = */ NULL, /* contents_size = */ 0);
+                        if (r < 0)
+                                return r;
+                }
 
-        if (!context->stdin) {
                 r = run_editor(context);
                 if (r < 0)
                         return r;
         }
 
+        _cleanup_close_ int stdin_data_fd = -EBADF;
+
         FOREACH_ARRAY(editfile, context->files, context->n_files) {
-                /* Always call strip_edit_temp_file which will tell if the temp file has actual changes */
-                r = strip_edit_temp_file(editfile);
+                if (context->stdin) {
+                        r = edit_file_install_one_stdin(editfile, stdin_data, stdin_size, &stdin_data_fd);
+                        if (r == 0) {
+                                log_notice("Stripped stdin content is empty, not writing file.");
+                                return 0;
+                        }
+                } else {
+                        r = edit_file_install_one(editfile);
+                        if (r == 0) {
+                                log_notice("%s: after editing, new contents are empty, not writing file.",
+                                           editfile->path);
+                                continue;
+                        }
+                }
                 if (r < 0)
                         return r;
-                if (r == 0) /* temp file doesn't carry actual changes, ignoring */
-                        continue;
-
-                r = RET_NERRNO(rename(editfile->temp, editfile->path));
-                if (r < 0)
-                        return log_error_errno(r,
-                                               "Failed to rename temporary file '%s' to target file '%s': %m",
-                                               editfile->temp,
-                                               editfile->path);
-                editfile->temp = mfree(editfile->temp);
 
                 log_info("Successfully installed edited file '%s'.", editfile->path);
         }