[PATCH v3 0/2] merge-ll: Cleanup merge driver temporaries after signal

[Jeff King <peff@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ý <mkoutny@suse.com>
         Reported-by: Jean Delvare <jdelvare@suse.de>
    +    Reported-by: Michal Koutný <mkoutny@suse.com>
         Signed-off-by: Jeff King <peff@peff.net>
     
      ## 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);