Subject: Re: [PATCH] wrapper: Fix a errno discrepancy on NetBSD.

From: Junio C Hamano <[email-masked]>
Date: Mon, 05 May 2025 08:43:18 -0700

Jeff King <[email-masked]> writes:

> That changed in cfea2f2da8 (packed-backend: check whether the
> "packed-refs" is regular file, 2025-02-28), which uses open_nofollow()
> to check for symlinks while we open it. But it feels like it would be
> more direct to just lstat() the file in the first place (which we end up
> doing anyway to check for other things besides symlinks!).
> ...
> It's not as "atomic" as open_nofollow() and fstat(), but I don't think
> we care about that for fsck. This is about consistency checking, not
> trying to beat races against active adversaries (not to mention that our
> open_nofollow() is best-effort anyway, and may be racy).

True.  Atomicity, which the use of open_nofollow() and fstat() tries
to achieve, may not matter in fsck.  We can think of the use of
open_nofollow() in this particular codepath merely as a convenient
helper function, and I do not think we have any problem with such a
helper function.

But open_nofollow() and its emulation can be called from other
codepaths that may care about atomicity, and I am not sure what our
attitude towards atomicity requirements vs platform capabilities
should be.

If an atomicity (or any other) requirement in a particular codepath
has a simple and obvious way to solve on common platforms, but that
the mechanism to implement the simple and obvious way is unavailable
on other platforms, where does it lead us?

For some kind of requirements, we can treat it merely as a quality
of implementation issue, similar to how finalize_object_file()
ideally wants to do the create(TMP) then link(TMP->FINAL) then
unlink(TMP) dance (because we want to detect collisions when able)
but has fallback implementation to create(TMP) then
rename(TMP->FINAL) (which punts on collision detection) on platforms
where the preferred way does not work.  It falls into this category,
I would think, to think of use of open_nofollow() in this codepath
as a mere helper function that makes the code in fsck shorter.

But for other kind of requirements, we want to fulfill them on all
platforms that we claim to support.  Using open_nofollow() to
achieve hard atomicity requirement would be a bug in such a
situation.  Should we somehow warn our developers against its use?

Idealists in us first try hard to find the right abstraction that
would work everywhere, and use compat/ layer to implement that
abstraction, but we of course are often not successful, and end up
with a series of #ifdef for pieces of platform-specific code in
fairly higher layer.  It feels that open_nofollow() that is not
necesarily atomic is the latter but that is done at a level that is
a bit too low.  I dunno.