[PATCH v2] find: fix -xdev -depth (and -delete)
Dominique Martinet
asmadeus at codewreck.org
Thu Oct 26 21:42:35 UTC 2023
From: Dominique Martinet <dominique.martinet at atmark-techno.com>
find -xdev with -depth would check for same_fs after the subdirectory
has been processed (because the check is done in the file/dir action,
which is evaluated too late in the -depth case)
This renders `find -xdev -delete` useless, as reported in 2012 here:
https://bugs.busybox.net/show_bug.cgi?id=5756
The bug report suggested adding an extra hook, which would be required
if we were to keep the current xdev approach that allows all filesystems
given in argument, but GNU findutils and OpenBSD find actually stop on
the first filesystem boundary e.g. for the following tree:
$ find test -exec stat --format "%d %n" {} +
27 test
27 test/file
59 test/tmpfs
27 test/tmpfs/bind
27 test/tmpfs/bind/file
59 test/tmpfs/file
(Where 'test/tmpfs' is a tmpfs, and 'test/tmpfs/bind' is a bind mount
to a neighboring directory in the same filesystem as 'test' -- also
tested with a symlink and -follow for openbsd which has no bind mount)
Then `find test test/tmpfs -xdev` does not print test/tmpfs/bind/file.
This makes the implementation much simpler (although it's a bit ugly to
carry the parent st_dev as an argument to the function) and smaller
code, and would allow for easy addition of rm/cp --one-file-system if
we want to do that later.
Note: this also no longer stores the stat result in 'status' in
recursive_action1 as that was not used and doing this saves 10 bytes
function old new delta
recursive_action1 361 385 +24
parse_params 1503 1510 +7
recursive_action 63 65 +2
fileAction 206 127 -79
find_main 520 417 -103
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 3/2 up/down: 33/-182) Total: -149 bytes
text data bss dec hex filename
78267 1582 1552 81401 13df9 busybox_old
78118 1582 1552 81252 13d64 busybox_unstripped
---
v1->v2:
properly initializes 'status' in recursive_action1 to avoid incorrectly
returing failure code with find -xdev -depth, as was reported on alpine:
https://gitlab.alpinelinux.org/alpine/aports/-/merge_requests/52406#note_348411
This isn't exactly the same patch I sent to alpine as this saves a few
bytes; my first version of the fix was just setting 'status' before the
goto in the xdev if branch.
findutils/find.c | 44 ++--------------------------------------
include/libbb.h | 1 +
libbb/recursive_action.c | 19 ++++++++++-------
3 files changed, 15 insertions(+), 49 deletions(-)
diff --git a/findutils/find.c b/findutils/find.c
index 31c9969886f6..a4a6bbc2df91 100644
--- a/findutils/find.c
+++ b/findutils/find.c
@@ -501,7 +501,6 @@ struct globals {
#endif
action ***actions;
smallint need_print;
- smallint xdev_on;
smalluint exitstatus;
recurse_flags_t recurse_flags;
IF_FEATURE_FIND_EXEC_PLUS(unsigned max_argv_len;)
@@ -1015,26 +1014,10 @@ static int FAST_FUNC fileAction(
struct stat *statbuf)
{
int r;
- int same_fs = 1;
-
-#if ENABLE_FEATURE_FIND_XDEV
- if (S_ISDIR(statbuf->st_mode) && G.xdev_count) {
- int i;
- for (i = 0; i < G.xdev_count; i++) {
- if (G.xdev_dev[i] == statbuf->st_dev)
- goto found;
- }
- //bb_error_msg("'%s': not same fs", fileName);
- same_fs = 0;
- found: ;
- }
-#endif
#if ENABLE_FEATURE_FIND_MAXDEPTH
if (state->depth < G.minmaxdepth[0]) {
- if (same_fs)
- return TRUE; /* skip this, continue recursing */
- return SKIP; /* stop recursing */
+ return TRUE; /* skip this, continue recursing */
}
if (state->depth > G.minmaxdepth[1])
return SKIP; /* stop recursing */
@@ -1051,11 +1034,6 @@ static int FAST_FUNC fileAction(
return SKIP;
}
#endif
- /* -xdev stops on mountpoints, but AFTER mountpoit itself
- * is processed as usual */
- if (!same_fs) {
- return SKIP;
- }
/* Cannot return 0: our caller, recursive_action(),
* will perror() and skip dirs (if called on dir) */
@@ -1295,7 +1273,7 @@ static action*** parse_params(char **argv)
#if ENABLE_FEATURE_FIND_XDEV
else if (parm == OPT_XDEV) {
dbg("%d", __LINE__);
- G.xdev_on = 1;
+ G.recurse_flags |= ACTION_XDEV;
}
#endif
#if ENABLE_FEATURE_FIND_MAXDEPTH
@@ -1718,24 +1696,6 @@ int find_main(int argc UNUSED_PARAM, char **argv)
G.actions = parse_params(&argv[firstopt]);
argv[firstopt] = NULL;
-#if ENABLE_FEATURE_FIND_XDEV
- if (G.xdev_on) {
- struct stat stbuf;
-
- G.xdev_count = firstopt;
- G.xdev_dev = xzalloc(G.xdev_count * sizeof(G.xdev_dev[0]));
- for (i = 0; argv[i]; i++) {
- /* not xstat(): shouldn't bomb out on
- * "find not_exist exist -xdev" */
- if (stat(argv[i], &stbuf) == 0)
- G.xdev_dev[i] = stbuf.st_dev;
- /* else G.xdev_dev[i] stays 0 and
- * won't match any real device dev_t
- */
- }
- }
-#endif
-
for (i = 0; argv[i]; i++) {
if (!recursive_action(argv[i],
G.recurse_flags,/* flags */
diff --git a/include/libbb.h b/include/libbb.h
index 0883fb565fa9..a206f5e884f4 100644
--- a/include/libbb.h
+++ b/include/libbb.h
@@ -514,6 +514,7 @@ enum {
ACTION_DEPTHFIRST = (1 << 3),
ACTION_QUIET = (1 << 4),
ACTION_DANGLING_OK = (1 << 5),
+ ACTION_XDEV = (1 << 6),
};
typedef uint8_t recurse_flags_t;
typedef struct recursive_state {
diff --git a/libbb/recursive_action.c b/libbb/recursive_action.c
index b1c4bfad7ccf..d4b40afdead3 100644
--- a/libbb/recursive_action.c
+++ b/libbb/recursive_action.c
@@ -62,13 +62,15 @@ static int FAST_FUNC true_action(struct recursive_state *state UNUSED_PARAM,
* ACTION_FOLLOWLINKS mainly controls handling of links to dirs.
* 0: lstat(statbuf). Calls fileAction on link name even if points to dir.
* 1: stat(statbuf). Calls dirAction and optionally recurse on link to dir.
+ *
+ * If ACTION_XDEV, stop on different filesystem _after_ it has been processed
*/
-static int recursive_action1(recursive_state_t *state, const char *fileName)
+static int recursive_action1(recursive_state_t *state, const char *fileName, dev_t parentDev)
{
struct stat statbuf;
unsigned follow;
- int status;
+ int status = TRUE;
DIR *dir;
struct dirent *next;
@@ -76,8 +78,7 @@ static int recursive_action1(recursive_state_t *state, const char *fileName)
if (state->depth == 0)
follow = ACTION_FOLLOWLINKS | ACTION_FOLLOWLINKS_L0;
follow &= state->flags;
- status = (follow ? stat : lstat)(fileName, &statbuf);
- if (status < 0) {
+ if ((follow ? stat : lstat)(fileName, &statbuf) < 0) {
#ifdef DEBUG_RECURS_ACTION
bb_error_msg("status=%d flags=%x", status, state->flags);
#endif
@@ -114,6 +115,10 @@ static int recursive_action1(recursive_state_t *state, const char *fileName)
return TRUE;
}
+ /* skip cross devices -- we still need to process action */
+ if ((state->flags & ACTION_XDEV) && parentDev != 0 && statbuf.st_dev != parentDev)
+ goto skip_recurse;
+
dir = opendir(fileName);
if (!dir) {
/* findutils-4.1.20 reports this */
@@ -121,7 +126,6 @@ static int recursive_action1(recursive_state_t *state, const char *fileName)
/* To trigger: "find -exec rm -rf {} \;" */
goto done_nak_warn;
}
- status = TRUE;
while ((next = readdir(dir)) != NULL) {
char *nextFile;
int s;
@@ -132,7 +136,7 @@ static int recursive_action1(recursive_state_t *state, const char *fileName)
/* process every file (NB: ACTION_RECURSE is set in flags) */
state->depth++;
- s = recursive_action1(state, nextFile);
+ s = recursive_action1(state, nextFile, statbuf.st_dev);
if (s == FALSE)
status = FALSE;
free(nextFile);
@@ -146,6 +150,7 @@ static int recursive_action1(recursive_state_t *state, const char *fileName)
}
closedir(dir);
+skip_recurse:
if (state->flags & ACTION_DEPTHFIRST) {
if (!state->dirAction(state, fileName, &statbuf))
goto done_nak_warn;
@@ -177,5 +182,5 @@ int FAST_FUNC recursive_action(const char *fileName,
state.fileAction = fileAction ? fileAction : true_action;
state.dirAction = dirAction ? dirAction : true_action;
- return recursive_action1(&state, fileName);
+ return recursive_action1(&state, fileName, 0);
}
--
2.41.0
More information about the busybox
mailing list