From: Elijah Newren <newren@gmail.com>
Date: Fri, 11 Sep 2026 18:06:43 GMT
Subject: Re: [PATCH v2 2/3] merge-ll: catch close() errors when writing external tempfiles
Message-ID: <CABPp-BG6wYkr4wjr-iqak9fYo4+49WvjROdZ_MK5=g27WcUmMA@mail.gmail.com>
In-Reply-To: <20260911171139.GB1610200@coredump.intra.peff.net>

On Fri, Sep 11, 2026 at 10:11 AM Jeff King <peff@peff.net> wrote:
>
> When writing out tempfiles for an external merge driver, we catch the
> case that write() fails, but not the follow-up close(). This close()
> would usually succeed, but the system could report a delayed write error
> (e.g., on a network file system).
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  merge-ll.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/merge-ll.c b/merge-ll.c
> index 5b6af15e23..5a11a9613b 100644
> --- a/merge-ll.c
> +++ b/merge-ll.c
> @@ -180,9 +180,9 @@ static void create_temp(mmfile_t *src, char *path, size_t len)
>
>         xsnprintf(path, len, ".merge_file_XXXXXX");
>         fd = xmkstemp(path);
> -       if (write_in_full(fd, src->ptr, src->size) < 0)
> +       if (write_in_full(fd, src->ptr, src->size) < 0 ||
> +           close(fd) < 0)
>                 die_errno("unable to write temp-file");
> -       close(fd);
>  }
>
>  /*
> --
> 2.56.0.rc0.314.g7a874b6915

I got tripped up at first on this patch; if write_in_full() < 0, then
we won't explicitly close(), but since die will result in an implicit
close, that's not a problem.

Instead, the only thing that changes is we also die if close() fails.

Looks good.