Re: [PATCH v3 10/13] rerere.c: move error_errno() closer to the source system call

From: Stefan Beller <sbeller@google.com>
Date: May 30, 2017, 18:58 UTC
Message-ID: <CAGZ79kayr3QkdQaVe3cmPyWa2mJQmBxhe=XBMzaLWkJU6U+zsA@mail.gmail.com>
In-Reply-To: <20170526033510.1793-11-gitster@pobox.com>

On Thu, May 25, 2017 at 8:35 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 22 quoted lines

> From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>
> We are supposed to report errno from fopen(). fclose() between fopen()
> and the report function could either change errno or reset it.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  rerere.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/rerere.c b/rerere.c
> index 1351b0c3fb..c26c29f87a 100644
> --- a/rerere.c
> +++ b/rerere.c
> @@ -489,8 +489,9 @@ static int handle_file(const char *path, unsigned char *sha1, const char *output
>         if (output) {
>                 io.io.output = fopen(output, "w");
>                 if (!io.io.output) {
> +                       error_errno("Could not write %s", output);
>                         fclose(io.input);
> -                       return error_errno("Could not write %s", output);
> +                       return -1;

and here we assume fclose doesn't error out.

This is ok as we are sure it cannot be EBADF as we just opened that fd and as we did not read from io.input yet, any errno from close(2), fflush(3) are not expected?

close(2) may yield an EINTR, in which case we may want to

  while (fclose(io.input) && errno != EINTR)
    ; /*do nothing*/

On the other hand this is already in the error path, so maybe we're ok leaking an fd here in case of a signal?