Bug 5756

-xdev is supposed to prevent recursing into mount points of other filesystems. This check is performed in fileAction in find.c, which returns SKIP when it finds a mount point of (or rather, any directory in) a different filesystem.

However, when -depth or -delete is specified, a depth-first search is done, meaning the mount point has already been recursed (and emptied with -delete!) by the time the xdev check is performed.

The following should illustrate the problem:

matthijs@grubby:~$ mkdir test
matthijs@grubby:~$ mkdir test/mountpoint
matthijs@grubby:~$ sudo mount -t tmpfs tmpfs test/mountpoint
matthijs@grubby:~$ touch test/mountpoint/file
matthijs@grubby:~$ find test
test
test/mountpoint
test/mountpoint/file
matthijs@grubby:~$ find test -xdev
test
test/mountpoint
matthijs@grubby:~$ busybox find test -xdev
test
test/mountpoint
matthijs@grubby:~$ find test -xdev -depth
test/mountpoint
test
matthijs@grubby:~$ busybox find test -xdev -depth
test/mountpoint/file
test/mountpoint
test

This was tested using git master (4d5955e9). Note that, without -depth, busybox and regular find behave normally. With -depth, busybox erronously shows test/mountpoint/file.


The cause for this is that both the xdev check happens in fileAction in find.c (which is used for both files and dirs), which is called _after_ recursing into the mountpoint with -depth, making the check effectively useless.


To fix this, I think checks that limit recursion must be done on the way down, regardless of -depth. This could be implemented by adding a dirCheck argument to the recursive_action function, that is always called before recursing.

To still allow the current "process the mountpoint but don't recurse into it", this function should be allowed the existing TRUE and SKIP values, but also a new DONTRECURSE value, which means to process the directory itself, but not recurse into it. I don't think the case of not processing a directory, but recursing into it (e.g., mindepth) needs to be explicitly supported here, since that can still be handled in the dirAction function as before.

If this dirCheck function is added, it would make sense to no longer allow dirAction to return SKIP, since all the users of recursive_action could be changed to use dirCheck instead of returning skip from dirAction.

OTOH, it might be cumbersome to do this, while ensuring the behaviour isn't changed (most of the "return SKIP;" statements are not at the top of the dirAction functions), and the code might not become more clear from this.

In addition, a quick grep suggests that the only code that uses both SKIP and DEPTHFIRST is find, so just leaving the other code untouched and adding a dirCheck argument is probably the best way to handle this.


I suspect that this same problem partially exists for the -max-depth option. However, since the max-depth option is checked on files, not just directories and since the action is not performed when max-depth is exceeded (unlike xdev, where the action is still performed on the first directory on the different filesystem (e.g., mountpoint)).

This means that using -depth and -max-depth currently makes Busybox find very inefficient, since it traverses the entire try, only to do nothing on every file that's too deep.

If the maxdepth check is also moved into the new dirCheck function, this inefficiency should also be solved at the same time.


If someone can comment on the above implementation suggestion, I'll be happy to have a stab at implementing it.