On Wed, 20 May 2026 at 12:22:50 +0200, Oswald Buddenhagen wrote:
> On Tue, May 19, 2026 at 12:31:51PM +1000, Seth McDonald wrote:
> > What are the reasons users would want to dry run a program? There's
> > curiosity (what will this do?), assurance (will this do what I want?),
>
> > and - most importantly - checking for potential problems in a safe and
> > non-destructive environment.
> >
> deferring an error report about an unwritable directory to a real run
> doesn't make it any less safe.
With the exception of changing a mkdir() call to mkdir_p(), the patch is
intentionally constrained to the behaviour of mkdir_p(). It does not at
all concern the behaviour of functions calling mkdir_p(), including what
they choose to do upon failure (such as simply giving an error report).
And that's a good thing. mkdir_p() should be a standalone logical unit
that can be used in situations other than those it was already used in.
My point here being that the write-perm check does more than not defer
an error report. mkdir_p() could be used in any which way in the
future. Such use should be able to depend on it returning identically
in both dry and real runs. And if it does rely on this, but then
mkdir_p() returns differently, any kind of logic bug may arise.
Or in short: it's good to future-proof mkdir_p() for later use,
particularly since it's as simple as a single call to faccessat().
> it's arguably a tiny bit less useful, but one
> has to consider the trade-off between implementation complexity and covering
> corner cases (unlikely and little practical impact).
I personally wouldn't consider three functions with a mean size of 18
LOC too complex. (Yes, LOC generally isn't a reliable unit of
measurement; I'm just saying the complexity isn't unreasonable.)
> > access() performs permission checks using the process' real user/group
> > IDs, which may be different to its effective user/group IDs.
> >
> no, they can't. mbsync isn't a setuid program. there is no security boundary
> here, and therefore no security-sensitive race condition.
This was just some extra future-proofing the implementation benefited
from. The main reason was the second given - it prevents misleading
conclusions and minimises race conditions by performing all the
existence, filetype, and search-permission checks at once with open().
(Added after further thought:)
That said, after re-reading the POSIX page on access(), it would be
possible to have access() also perform the filetype check by
keeping/appending a slash at the end of each checked path. So we might
be able to call access( F_OK ) on each path component (with the trailing
shash) to find the first existent component and then call access( W_OK )
on that.
A downside to this approach is it opens a new race condition. With
open() + faccessat( W_OK ) you can be certain faccessat() is checking
the same directory that was open()ed. While with access( F_OK ) +
access( W_OK ) there's the chance the directory was renamed or similarly
altered in between the two calls.
I personally still prefer open() + faccessat( W_OK ) since it's more
robust, but I'm open to using access( F_OK ) + access( W_OK ).
Take care,
Seth McDonald.
--
E9D1 26A5 F0D4 9DF7 792B C2E2 B4BF 4530 D39B 2D51
_______________________________________________
isync-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/isync-devel
