coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: fstatat + AT_NO_AUTOMOUNT


From: Pádraig Brady
Subject: Re: fstatat + AT_NO_AUTOMOUNT
Date: Mon, 7 Mar 2022 21:56:17 +0000
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:97.0) Gecko/20100101 Thunderbird/97.0

On 07/03/2022 19:09, Paul Eggert wrote:
On 3/7/22 09:33, Pádraig Brady wrote:
On 07/03/2022 17:11, Paul Eggert wrote:
On 3/7/22 07:02, Pádraig Brady wrote:
When looking into https://bugs.gnu.org/54286
which discussed statx + AT_NO_AUTOMOUNT,
I was wondering about the fstatat changes in
https://github.com/coreutils/coreutils/commit/571f63f50
and whether some of those should also specify AT_NO_AUTOMOUNT?

I'm a bit lost, as I see no mention of fstatat in that commit.

That commit replaced some stat() calls with fstatat().

How did it do that? Sorry, I'm still lost: the commit doesn't say
"fstatat" anywhere.

$ git show 571f63f50 | grep -B3 fstatat
-    (dereference_dest_dir_symlinks ? stat (file, &st) : lstat (file, &st));
-  int err = (stat_result == 0 ? 0 : errno);
+  int flags = dereference_dest_dir_symlinks ? 0 : AT_SYMLINK_NOFOLLOW;
+  int err = errnoize (fstatat (AT_FDCWD, file, &st, flags));
--
-      if ((logical ? stat (source, &source_stats)
-           : lstat (source, &source_stats))
-          != 0)
+      source_status = fstatat (AT_FDCWD, source, &source_stats, nofollow_flag);

Attached is an adjustment for ln in coreutils at least.

I see a problem with that change. Suppose D does not exist, but is an
on-demand directory such as used for automounter indirect maps. Then "ln
-f S D" will use fstatat with AT_NO_AUTOMOUNT on D, and will assume that
D does not exist.

I can see where AT_NO_AUTOMOUNT might be useful for commands like 'find'
that do mass lstats or commands like 'df' designed to access
mountpoints, but why is it useful for 'ln'? It seems like it'd be more
trouble than it's worth.

Agreed. As a rule of thumb I guess for stateful commands
it would be best not to use AT_NO_AUTOMOUNT

I see unlinkat() uses lstatat(),
and in that context it would be better to specify AT_NO_AUTOMOUNT
I think since we're operating on the dir not the file itself.

If it weren't for on-demand directories I would agree with
AT_NO_AUTOMOUNT there, but I think the reason would be different.
Ordinarily mount points are directories, and so automounting replaces
one directory with another. Since gnulib/lib/unlinkat.c is trying to
determine whether it's unlinking a directory, mounting won't change
that, so there's no point to mounting, and AT_NO_AUTOMOUNT is a
performance win there. (Although it's possible to mount onto a
non-directory, this is bad practice and I don't think we need to worry
about that.)

Unfortunately, on-demand directories mess up the calculation, since they
replace nonexistent files with directories. So it appears that
AT_NO_AUTOMOUNT would not be an acceptable optimization in
gnulib/lib/unlinkat.c, since it would mess up in that case.

agreed

Also agreed that eliminating these aliases
would avoid at least this particular ambiguity.

Yes, that looks like a win (unfortunately, as the aliases do make things
shorter; but they're too confusing now).

cheers,
Pádraig



reply via email to

[Prev in Thread] Current Thread [Next in Thread]