Re: fstatat + AT_NO_AUTOMOUNT

[Pádraig Brady]

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