On Mon, 18 May 2026 at 10:42:47 +0200, Oswald Buddenhagen via isync-devel wrote:
> - it seems unnecessary to check the write permission.
mkdir() requires write permission on the parent directory to succeed.
> dry run doesn't need
> to ensure that a real run would actually succeed.
Strong disagree.
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.
That third reason is probably the one I use the most when dry running.
If I'm unsure or not confident that a program will work, and I know the
program has the ability to alter persistent data, I will perform a dry
run to check for problems with the guarantee that no persistent data
will be altered. I do _not_ want issues to prop up in a real run that
could have been easily identified in a dry run so they could be fixed
prior to the real run. Allowing this to occur is reckless; it
needlessly forces such problems to occur in a comparably unsafe
environment.
Currently the dry run behaviour of mbsync is destructive. By creating
directories with mkdir(), it is modifying the persistent state of the
filesystem and hence destroying its previous state. What if I, as a
user, have a good reason for a directory not yet existing? I would
reasonably expect a dry run to retain that state and would consider it a
bug to change it.
> such a check is
> inherently racy anyway, as hypothetically the situation on the ground
> could change in between.
Indeed, hence mkdir_p_check() includes in its function comment:
| * Note that a successful call to this function does NOT guarantee a later call
| * to mkdir_p() will be successful.
> - opening the directory and the subsequent faccessat() seem rather
> over-engineered to me. a simple access() would be good enough in this
> situation.
The reason for the open() + faccessat() instead of just access() is two
fold.
For one, functions like mkdir() determine whether the process has
sufficient permissions to succeed using its effective user/group IDs.
So to accurately simulate its behaviour, we need to take this into
account.
access() performs permission checks using the process' real user/group
IDs, which may be different to its effective user/group IDs. While
faccessat() allows for the AT_EACCESS flag, which changes the behaviour
to use the effective IDs. open() also uses the effective IDs, which is
used here to check for search permissions.
And for two, simply checking for write permission on each directory
component of the given path can be misleading. For example, suppose we
have the following directory structure:
/A/B/C
A drwxr-xr-x
B drwxrwxrwx
C drwxr-xr--
Suppose we're running the process with a user ID (effective and real)
that falls under the 'group' category for these directories. And
suppose we want to mkdir( /A/B/C/D ). This would fail, since /A/B/C
does not grant us write permissions.
But if we were to simulate it by checking each directory component with
access( "/.../", W_OK ), first /A/B/C/D would fail, then /A/B/C would
fail, but then /A/B would succeed, giving the impression that we could
mkdir( /A/B/C ) then mkdir( /A/B/C/D ) and it would work. Sure, we
could check for both F_OK and W_OK for each directory component, but
this allows for _even more_ of the race conditions you expressed concern
about (and that I would also like to minimise).
> - once simplified, it should be possible to just have a single conditional
> in mkdir_p() instead of branching out to a whole separate call chain.
Hopefully it should now be more clear why I made it a separate function.
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
