From: Jeff King Date: Tue, 29 Sep 2026 05:12:00 GMT Subject: [PATCH v3 0/2] merge-ll: Cleanup merge driver temporaries after signal Message-ID: <20260929051200.GA1100000@coredump.intra.peff.net> In-Reply-To: <20260911171044.GA1609692@coredump.intra.peff.net> Here's a revised version of the series to switch merge-ll to use tempfile structs. Sorry, I got derailed a bit by travel. I dropped the v2 cleanup patch to use strbuf_read() for now. It was not strictly related and I think there's a bit of a rabbit hole that extends even beyond this function. That might become its own series later. Beyond that, this is mostly the same as v2. I tweaked the error-checking for close() in the first patch so that it's more obviously correct (and can produce a slightly more informative message). The range diff is below, though it's IMHO not very informative. The drop of the cleanup patch a lot of uninteresting textual ripples. [1/2]: merge-ll: catch close() errors when writing external tempfiles [2/2]: merge-ll: use tempfile API for external driver files merge-ll.c | 51 +++++++++++++++++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 18 deletions(-) 1: 62b4ac5ae0 < -: ---------- merge-ll: use strbuf to read back external merge result 2: 020e3bfcbd < -: ---------- merge-ll: catch close() errors when writing external tempfiles -: ---------- > 1: c6a4b3146d merge-ll: catch close() errors when writing external tempfiles 3: 914fafcd88 ! 2: b85e169cb3 merge-ll: use tempfile API for external driver files @@ Commit message When there's a long(er) running merge driver helper, the user may just decide to terminate it with Ctrl+C. That sends a signal to the driver - prog and to the whole process group as well, including the git merge + program and to the whole process group as well, including the git merge command proper. Hence the cleanup code would not run and .merge_file_* files are left behind. @@ Commit message So let's take the most conservative route, and just continue reporting the relative paths. - Commit-message-stolen-from: Michal Koutný Reported-by: Jean Delvare + Reported-by: Michal Koutný Signed-off-by: Jeff King ## merge-ll.c ## @@ merge-ll.c: static struct ll_merge_driver ll_merge_drv[] = { - - xsnprintf(path, len, ".merge_file_XXXXXX"); - fd = xmkstemp(path); -- if (write_in_full(fd, src->ptr, src->size) < 0 || -- close(fd) < 0) +- if (write_in_full(fd, src->ptr, src->size) < 0) +- die_errno(_("unable to write %s"), path); +- if (close(fd) < 0) +- die_errno(_("unable to close %s"), path); + struct tempfile *t = xmks_tempfile(".merge_file_XXXXXX"); -+ if (write_in_full(t->fd, src->ptr, src->size) < 0 || -+ close_tempfile_gently(t) < 0) - die_errno("unable to write temp-file"); ++ if (write_in_full(t->fd, src->ptr, src->size) < 0) ++ die_errno(_("unable to write %s"), get_tempfile_path(t)); ++ if (close_tempfile_gently(t) < 0) ++ die_errno(_("unable to close %s"), get_tempfile_path(t)); + return t; +} + @@ merge-ll.c: static enum ll_merge_result ll_ext_merge(const struct ll_merge_drive struct strbuf cmd = STRBUF_INIT; const char *format = fn->cmdline; struct child_process child = CHILD_PROCESS_INIT; -- int status, i; -+ int status; - struct strbuf result_buf = STRBUF_INIT; +- int status, fd, i; ++ int status, fd; + struct stat st; enum ll_merge_result ret; assert(opts); @@ merge-ll.c: static enum ll_merge_result ll_ext_merge(const struct ll_merge_driver *fn, @@ merge-ll.c: static enum ll_merge_result ll_ext_merge(const struct ll_merge_drive strbuf_addf(&cmd, "%d", marker_size); else if (skip_prefix(format, "P", &format)) @@ merge-ll.c: static enum ll_merge_result ll_ext_merge(const struct ll_merge_driver *fn, + child.use_shell = 1; strvec_push(&child.args, cmd.buf); status = run_command(&child); - -- if (strbuf_read_file(&result_buf, temp[1], 0) >= 0) { -+ if (strbuf_read_file(&result_buf, get_tempfile_path(tmp_a), 0) >= 0) { - result->size = result_buf.len; - result->ptr = strbuf_detach(&result_buf, NULL); - } - +- fd = open(temp[1], O_RDONLY); ++ fd = open(get_tempfile_path(tmp_a), O_RDONLY); + if (fd < 0) + goto bad; + if (fstat(fd, &st)) +@@ merge-ll.c: static enum ll_merge_result ll_ext_merge(const struct ll_merge_driver *fn, + close_bad: + close(fd); + bad: - for (i = 0; i < 3; i++) - unlink_or_warn(temp[i]); + delete_tempfile(&tmp_o);