[Bug 5756] New: find -xdev is effectively ignored when combined with -depth (or -delete)

[bugzilla at busybox.net]

https://bugs.busybox.net/show_bug.cgi?id=5756

           Summary: find -xdev is effectively ignored when combined with
                    -depth (or -delete)
           Product: Busybox
           Version: 1.19.x
          Platform: PC
        OS/Version: Linux
            Status: NEW
          Severity: minor
          Priority: P5
         Component: Standard Compliance
        AssignedTo: unassigned at busybox.net
        ReportedBy: matthijs at stdin.nl
                CC: busybox-cvs at busybox.net
   Estimated Hours: 0.0


-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 at grubby:~$ mkdir test
matthijs at grubby:~$ mkdir test/mountpoint
matthijs at grubby:~$ sudo mount -t tmpfs tmpfs test/mountpoint
matthijs at grubby:~$ touch test/mountpoint/file
matthijs at grubby:~$ find test
test
test/mountpoint
test/mountpoint/file
matthijs at grubby:~$ find test -xdev
test
test/mountpoint
matthijs at grubby:~$ busybox find test -xdev
test
test/mountpoint
matthijs at grubby:~$ find test -xdev -depth
test/mountpoint
test
matthijs at 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.

-- 
Configure bugmail: https://bugs.busybox.net/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.