Re: fstatat + AT_NO_AUTOMOUNT

2022-03-09 Thread Andreas Schwab
On Mär 09 2022, Paul Eggert wrote:

> I audited gnulib's uses of fstatat and found one fishy one that doesn't
> use AT_NO_AUTOMOUNT, namely, in fts.c where the follow-symlink branch uses
> 'stat' whereas the no-follow-symlink branch uses fstatat without
> AT_NO_AUTOMOUNT. I installed the first patch to cause it be consistent in
> using AT_NO_AUTOMOUNT, which is also consistent with what glibc does

??? In glibc, stat is the same as fstatat(,,,0).

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."



Re: fstatat + AT_NO_AUTOMOUNT

2022-03-09 Thread Paul Eggert
I audited gnulib's uses of fstatat and found one fishy one that doesn't 
use AT_NO_AUTOMOUNT, namely, in fts.c where the follow-symlink branch 
uses 'stat' whereas the no-follow-symlink branch uses fstatat without 
AT_NO_AUTOMOUNT. I installed the first patch to cause it be consistent 
in using AT_NO_AUTOMOUNT, which is also consistent with what glibc does 
(though this doesn't necessarily mean it's right - perhaps fts should 
have a new flag to control automounts, depending on what the user wants).


The second attached patch deprecates statat and lstatat, due to the 
confusion already mentioned.


I haven't audited Gnulib's uses of 'stat' and 'lstat'.From 44f347ce4009cd0100d0e6562939a032b16d6db1 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Wed, 9 Mar 2022 11:54:13 -0800
Subject: [PATCH 1/2] fts: be consistent about AT_NO_AUTOMOUNT
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* lib/fts.c (fts_stat): Use fstatat with AT_NO_AUTOMOUNT
consistently, instead of sometimes using stat (which implies
AT_NO_AUTOMOUNT) and sometimes using fstatat without AT_NO_AUTOMOUNT.
Remove a goto while we’re at it.
---
 ChangeLog |  8 
 lib/fts.c | 34 +-
 2 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index e3f0ed216c..58873a1762 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2022-03-09  Paul Eggert  
+
+	fts: be consistent about AT_NO_AUTOMOUNT
+	* lib/fts.c (fts_stat): Use fstatat with AT_NO_AUTOMOUNT
+	consistently, instead of sometimes using stat (which implies
+	AT_NO_AUTOMOUNT) and sometimes using fstatat without AT_NO_AUTOMOUNT.
+	Remove a goto while we’re at it.
+
 2022-03-07  Pádraig Brady  
 
 	fcntl-h: add AT_NO_AUTOMOUNT
diff --git a/lib/fts.c b/lib/fts.c
index 706c56c597..a1a7c09fdb 100644
--- a/lib/fts.c
+++ b/lib/fts.c
@@ -1766,7 +1766,8 @@ fts_stat(FTS *sp, register FTSENT *p, bool follow)
 {
 struct stat *sbp = p->fts_statp;
 
-if (p->fts_level == FTS_ROOTLEVEL && ISSET(FTS_COMFOLLOW))
+if (ISSET (FTS_LOGICAL)
+|| (ISSET (FTS_COMFOLLOW) && p->fts_level == FTS_ROOTLEVEL))
 follow = true;
 
 /*
@@ -1774,22 +1775,21 @@ fts_stat(FTS *sp, register FTSENT *p, bool follow)
  * a stat(2).  If that fails, check for a non-existent symlink.  If
  * fail, set the errno from the stat call.
  */
-if (ISSET(FTS_LOGICAL) || follow) {
-if (stat(p->fts_accpath, sbp)) {
-if (errno == ENOENT
-&& lstat(p->fts_accpath, sbp) == 0) {
-__set_errno (0);
-return (FTS_SLNONE);
-}
-p->fts_errno = errno;
-goto err;
-}
-} else if (fstatat(sp->fts_cwd_fd, p->fts_accpath, sbp,
-   AT_SYMLINK_NOFOLLOW)) {
-p->fts_errno = errno;
-err:memset(sbp, 0, sizeof(struct stat));
-return (FTS_NS);
-}
+int flags = (follow ? 0 : AT_SYMLINK_NOFOLLOW) | AT_NO_AUTOMOUNT;
+if (fstatat (sp->fts_cwd_fd, p->fts_accpath, sbp, flags) < 0)
+  {
+if (follow && errno == ENOENT
+&& 0 <= fstatat (sp->fts_cwd_fd, p->fts_accpath, sbp,
+ AT_SYMLINK_NOFOLLOW | AT_NO_AUTOMOUNT))
+  {
+__set_errno (0);
+return FTS_SLNONE;
+  }
+
+p->fts_errno = errno;
+memset (sbp, 0, sizeof *sbp);
+return FTS_NS;
+  }
 
 if (S_ISDIR(sbp->st_mode)) {
 if (ISDOT(p->fts_name)) {
-- 
2.35.1

From eea9688d521634c58efa81130e509f647bbd9ff9 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Wed, 9 Mar 2022 13:54:53 -0800
Subject: [PATCH 2/2] statat: now obsolete
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* lib/openat.h (statat, lstatat): Now deprecated.
All uses removed, and replaced with fstatat.
* modules/statat: Mark as obsolete, because it’s confusing:
it’s not clear whether it should use AT_NO_AUTOMOUNT,
which is implied by stat and by lstat, but not by fstatat.
* tests/test-statat.c: Disable deprecated-declarations warnings.
---
 ChangeLog   |  8 
 NEWS|  3 +++
 lib/fchownat.c  |  2 +-
 lib/openat.h|  2 ++
 lib/renameatu.c | 15 ---
 lib/unlinkat.c  |  5 +++--
 modules/fchownat|  1 -
 modules/renameatu   |  1 -
 modules/statat  |  7 +++
 modules/unlinkat|  1 -
 tests/test-statat.c |  4 
 11 files changed, 36 insertions(+), 13 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 58873a1762..294f6286f3 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,13 @@
 2022-03-09  Paul Eggert  
 
+	statat: now obsolete
+	* lib/openat.h (statat,