Re: [PATCH] strbuf_read_file(): preserve errno across close() call

[Junio C Hamano <gitster@xxxxxxxxx>]

René Scharfe <l.s.r@xxxxxx> writes:

> +#define IGNORE_ERROR(expr) do { int e_ = errno; expr; errno = e_; } while (0)

The macro certainly is a cute idea, but ...

> @@ -391,7 +393,7 @@ ssize_t strbuf_read(struct strbuf *sb, int fd, size_t hint)
>  
>  		if (got < 0) {
>  			if (oldalloc == 0)
> -				strbuf_release(sb);
> +				IGNORE_ERROR(strbuf_release(sb));
>  			else
>  				strbuf_setlen(sb, oldlen);
>  			return -1;

... ideally, I would imagine that we wish we could write this hunk
to something that expands to:

		if (got < 0) {
			do {
                                int e_ = errno;
                                if (oldalloc == 0)
                                        strbuf_release(sb);
                                else
                                        strbuf_setlen(sb, oldlen);
                                errno = e_;
			} while (0);
			return -1;

no?  That is (1) we do not want to rely too much on knowing that
strbuf_setlen() is very thin and does not touch errno, and hence (2)
we want to mark not just a single expr but a block as "we know we
got an error and errno from that error is more precious than what we
do in this block to clean thihngs up".

Of course, a pair of macros

	#define IGNORE_ERROR_BEGIN do { int e_ = errno
	#define IGNORE_ERROR_END errno = e_; } while (0)

is probably the only way to do so in C, and that is already too ugly
to live, so we cannot achieve the ideal.

So I dunno..

> @@ -617,9 +619,11 @@ ssize_t strbuf_read_file(struct strbuf *sb, const char *path, size_t hint)
>  	if (fd < 0)
>  		return -1;
>  	len = strbuf_read(sb, fd, hint);
> -	close(fd);
> -	if (len < 0)
> +	if (len < 0) {
> +		IGNORE_ERROR(close(fd));
>  		return -1;
> +	}
> +	close(fd);
>  
>  	return len;
>  }