[prev in list] [next in list] [prev in thread] [next in thread]
List: busybox
Subject: [PATCH] find: fix -xdev -depth (and -delete)
From: Dominique Martinet <asmadeus () codewreck ! org>
Date: 2023-09-19 8:17:47
Message-ID: 20230919081747.3136748-1-asmadeus () codewreck ! org
[Download RAW message or body]
From: Dominique Martinet <dominique.martinet@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.
function old new delta
recursive_action1 398 425 +27
parse_params 1660 1670 +10
recursive_action 70 72 +2
fileAction 216 127 -89
find_main 540 442 -98
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 3/2 up/down: 39/-187) Total: -148 bytes
text data bss dec hex filename
75774 2510 1552 79836 137dc busybox_old
75626 2510 1552 79688 13748 busybox_unstripped
---
This has biten me recently and seems to be a known issue for a while.
I don't recall seeing any patch for this since I've been on the list so
I took a quick stab, please take a look when you have time.
file system loops aside, this can be tested on linux with the following
"script":
++++++++++++++
cd $(mktemp -d bb-find-xdev.XXXXXX) || exit
mkdir tmpfs
mount -t tmpfs tmpfs tmpfs
mkdir -p a/b/c
touch file tmpfs/file a/b/c/file
find $PWD -xdev -delete
# ^ errors about tmpfs busy and pwd not empty,
# but tmpfs/file is left intact and everyting else is gone
umount tmpfs
find $PWD -xdev -delete
+++++++++++++
findutils/find.c | 44 ++--------------------------------------
include/libbb.h | 1 +
libbb/recursive_action.c | 13 +++++++++---
3 files changed, 13 insertions(+), 45 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 eb97a988045d..e8a7b449aef6 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..76dd664369a2 100644
--- a/libbb/recursive_action.c
+++ b/libbb/recursive_action.c
@@ -62,9 +62,11 @@ 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;
@@ -114,6 +116,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 */
@@ -132,7 +138,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 +152,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 +184,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.39.2
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic