Re: ERANGE strikes again on my Windows build; RFH

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 30, 2019, 17:42 UTC
Message-ID: <xmqqh81hhfqn.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <6eb02a73-9dcb-f1fc-f015-80e71e9910d6@kdbg.org>

Johannes Sixt <j6t@kdbg.org> writes:

Show 13 quoted lines

> In sha1-file.c:read_object_file_extended() we have the following pattern:
>
> 	errno = 0;
> 	data = read_object(r, repl, type, size);
> 	if (data)
> 		return data;
>
> 	if (errno && errno != ENOENT)
> 		die_errno(_("failed to read object %s"), oid_to_hex(oid));
>
> That is, it is expected that read_object() does not change the value of
> errno in the non-error case. I find it intriguing that we expect a quite
> large call graph that is behind read_object() to behave this way.

Yeah, that is somewhat unfortunate and dubious, but as long as it works (i.e. various system library calls the codepaths involved behave sensibly), ...

Show 11 quoted lines

> What if a subordinate callee starts doing
>
> 	if (some_syscall(...) < 0) {
> 		if (errno == EEXIST) {
> 			/* never mind, that's OK */
> 			...
> 		}
> 	}
>
> Would it be required to reset errno to its previous value in this
> failure-is-not-an-error case?

... that would be the logical conclusion, I think.

As a longer term fix for better portability (i.e. system libraries may not behave exactly the way how the codepaths and POSIX expects wrt to the errors detected), it may become necessary to change the function to yield "how the call failed" information in addition to "here is the block of bytes I read for the object", without relying on particular errno. But as a shorter term solution, ...

> The problem in my Windows build is that one of these subordinate
> syscalls is vsnprintf() (as part of a strbuf_add variant, I presume),
> and it fails with ERANGE when the buffer is too short. Do I have to
> modify the vsnprintf emulation to restore errno?

... I think this is a reasonable thing to do regardless.

Thanks.