On 9/29/18 9:30 AM, James Youngman wrote:
> Having a "don't update atime" option that only works sometimes (when I
> own the directory) seems problematic to me.
>
> If the user specifies the "keep access time unchanged" option and we
> can't honour it (because there is at least one directory that they
> don't own) what should we do?
>
> We can't simply abort at the time the error is detected, since we have
> searched some of the file system, and possibly generated some side
> effects. But perhaps the user would not want us to continue
> either. Reserving the option doesn't seem especially helpful, either
> (and may not work over NFS or other remote filesystems where root is
> squashed).
>
> In short, I can't see a way of making such an option non-user-surprising.
So regarding gnulib, there are 2 alternatives:
a) rename FTS_NOATIME to FTS_NOATIME, and add a retry to open/openat
in each place when that was using O_NOATIME and we got EPERM.
b) remove FTS_NOATIME - the attached would do that ... in the naive
hope that no other project has picked it up yet (which I doubt).
I'm 60:40 pro b).
Comments?
Have a nice day,
Berny
From b1bc3c1c6e97fc53fa813575611d84a0295d5e5d Mon Sep 17 00:00:00 2001
From: Bernhard Voelker <m...@bernhard-voelker.de>
Date: Sat, 29 Sep 2018 11:17:13 +0200
Subject: [PATCH] fts: remove FTS_NOATIME
This reverts commit da4d6974013c822af1498941e32db774b2031765.
We cannot guarantee that O_NOATIME works: e.g. openat fails
with EPERM if the effective user ID of the caller does not match
the owner of the file and the caller is not privileged.
Downstream findutils has never picked up FTS_NOATIME.
* lib/fts_.h (FTS_NOATIME): Remove bit flag.
(FTS_OPTIONMASK): Adjust.
* lib/fts.c (diropen, fts_open, fts_build): Likewise.
(fd_ring_check): Likewise.
---
ChangeLog | 13 +++++++++++++
lib/fts.c | 11 ++++-------
lib/fts_.h | 2 +-
3 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 03aab337a..bf06f4318 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,16 @@
+2018-09-29 Bernhard Voelker <m...@bernhard-voelkerb.de>
+
+ fts: remove FTS_NOATIME
+ This reverts commit da4d6974013c822af1498941e32db774b2031765.
+ We cannot guarantee that O_NOATIME works: e.g. openat fails
+ with EPERM if the effective user ID of the caller does not match
+ the owner of the file and the caller is not privileged.
+ Downstream findutils has never picked up FTS_NOATIME.
+ * lib/fts_.h (FTS_NOATIME): Remove bit flag.
+ (FTS_OPTIONMASK): Adjust.
+ * lib/fts.c (diropen, fts_open, fts_build): Likewise.
+ (fd_ring_check): Likewise.
+
2018-09-27 Akim Demaille <a...@lrde.epita.fr>
timevar: import from Bison.
diff --git a/lib/fts.c b/lib/fts.c
index 1ccc78c11..aaa6bb293 100644
--- a/lib/fts.c
+++ b/lib/fts.c
@@ -370,8 +370,7 @@ internal_function
diropen (FTS const *sp, char const *dir)
{
int open_flags = (O_SEARCH | O_CLOEXEC | O_DIRECTORY | O_NOCTTY | O_NONBLOCK
- | (ISSET (FTS_PHYSICAL) ? O_NOFOLLOW : 0)
- | (ISSET (FTS_NOATIME) ? O_NOATIME : 0));
+ | (ISSET (FTS_PHYSICAL) ? O_NOFOLLOW : 0));
int fd = (ISSET (FTS_CWDFD)
? openat (sp->fts_cwd_fd, dir, open_flags)
@@ -426,8 +425,7 @@ fts_open (char * const *argv,
early, doing it here saves us the trouble of ensuring
later (where it'd be messier) that "." can in fact
be opened. If not, revert to FTS_NOCHDIR mode. */
- int fd = open (".",
- O_SEARCH | (ISSET (FTS_NOATIME) ? O_NOATIME : 0));
+ int fd = open (".", O_SEARCH);
if (fd < 0)
{
/* Even if "." is unreadable, don't revert to FTS_NOCHDIR mode
@@ -1304,8 +1302,7 @@ set_stat_type (struct stat *st, unsigned int dtype)
(((ISSET(FTS_PHYSICAL) \
&& ! (ISSET(FTS_COMFOLLOW) \
&& cur->fts_level == FTS_ROOTLEVEL)) \
- ? O_NOFOLLOW : 0) \
- | (ISSET (FTS_NOATIME) ? O_NOATIME : 0)), \
+ ? O_NOFOLLOW : 0)), \
Pdir_fd)
/*
@@ -1796,7 +1793,7 @@ fd_ring_check (FTS const *sp)
int fd = i_ring_pop (&fd_w);
if (0 <= fd)
{
- int open_flags = O_SEARCH | O_CLOEXEC | O_NOATIME;
+ int open_flags = O_SEARCH | O_CLOEXEC;
int parent_fd = openat (cwd_fd, "..", open_flags);
if (parent_fd < 0)
{
diff --git a/lib/fts_.h b/lib/fts_.h
index 70cc9e3d6..6188122fd 100644
--- a/lib/fts_.h
+++ b/lib/fts_.h
@@ -149,7 +149,7 @@ typedef struct {
dirent.d_type data. */
# define FTS_DEFER_STAT 0x0400
-# define FTS_NOATIME 0x0800 /* use O_NOATIME during traversal */
+/* 0x0800 unused, was non-working FTS_NOATIME */
/* Use this flag to disable stripping of trailing slashes
from input path names during fts_open initialization. */
--
2.19.0
