Re: [PATCH v2 2/3] refs/files-backend: stop setting errno from lock_ref_oid_basic

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 29, 2021, 01:55 UTC
Message-ID: <xmqqa6pham83.fsf@gitster.g>
In-Reply-To: <CAFQ2z_O=4sUjh1wk6nRijp9Gz2eeqX4=EY+Q-OTi9ppb9ikg3g@mail.gmail.com>

Han-Wen Nienhuys <hanwen@google.com> writes:

Show 11 quoted lines

> On Wed, Apr 28, 2021 at 6:20 AM Junio C Hamano <gitster@pobox.com> wrote:
>> > These calls do I/O and therefore clobber errno. They are not inspecting the
>> > incoming errno.
>>
>> Hmph, are you saying that these calls do I/O and always the I/O
>> would fail?  A system call that is successfull don't touch errno;
>> only the calls that resulted in failure do.
>
> I'm saying that callers cannot reliably observe the errno result of
> lock_ref_oid_basic, because it might be clobbered by a failing
> follow-up call.

Sorry, I still do not quite get it. For example, you cite that a call to lock_ref_oid_basic() in files_create_symref() is followed by create_symref_locked() that may clobber errno when the latter fails.

But a failing lock_ref_oid_basic() would yield NULL and causes the caller to leave, before calling create_symref_locked() and letting it clobber errno, and the caller of files_create_symref() can observe, when it returns -1 to signal an error, the errno left by lock_ref_oid_basic(), no? I would understand it if no caller of files_create_symref() cares what is in errno when it receives negative return to signal a failure, though.

And when lock_ref_oid_basic() did not fail, create_symref_locked() calls helpers that can fail (e.g. fdopen_lock_file()) and result in errno getting updated to record how it failed (this is also reported to the user via "error(... strerror(errno))").

So a caller of files_create_symref() may not be able to tell between lock_ref_oid_basic() and create_symref_locked() which one caused the files_create_symref() call to fail, but in either case it should be able to inspect errno to learn what kind of error we got from the underlying system, no?