From: Mike Yuan Date: Sun, 11 Aug 2024 13:41:07 +0000 (+0200) Subject: edit-util: several cleanups for --stdin handling X-Git-Tag: v257-rc1~712^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=40f5c372c22d2dea20c2d5155754faec9c4460cd;p=thirdparty%2Fsystemd.git edit-util: several cleanups for --stdin handling 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. --- diff --git a/src/shared/edit-util.c b/src/shared/edit-util.c index b1f2aade275..5dd8595a369 100644 --- a/src/shared/edit-util.c +++ b/src/shared/edit-util.c @@ -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); }