[Toybox] [PATCH] find.test: fix test expectation for BSD libcs.

enh enh at google.com
Wed Aug 28 14:55:49 PDT 2019


On Wed, Aug 28, 2019 at 2:44 PM Rob Landley <rob at landley.net> wrote:
>
> On 8/26/19 11:20 PM, enh wrote:
> > this comment had me worried:
> >
> > "Tweak DIRTREE_STATLESS so it returns zero stat for any error (I'm testing
> > that dev, ino, and blksize are all zero), and fill in file type from readdir()"
> >
> > and indeed, this breaks one of the find(1) tests i added. (and is how
> > we reopened this conversation about ls(1) --- the difference between
> > find which only ignores ENOENT and ls which ignores more.)
>
> Except the error I'm getting on my dir test isn't ENOENT, it's "permission
> denied" which is EACCES.

that's ls, not find.

> So which errors should it ignore? I changed it because only a couple things are
> using DIRTREE_STATLESS.

but like i said, find and ls are demonstrably different in which errno
values they care about.

> Sigh, I don't have test cases for this.

(there are some for find --- that's what your patch broke :-) )

> Which specific error cases are what
> here? According to man 2 stat, we got:
>
> EACCES (this), EBADF (pilot error), EFAULT (pilot error), ELOOP (TODO: test),
> ENAMETOOLONG (equivalent to ENOENT, sort of ECANTENT), ENOMEM(pilot error),
> ENOTDIR (ECANTENT), EOVERFLOW (doesn't apply to us), EBADF (pilot error), EINVAL
> (pilot error), and a different ENOTDIR because of dirfd (pilot error except
> maybe the nocwd "." has been deleted case?).
>
> You know, what it sounds like is DIRTREE_STATLESS needs to stick errno in one of
> the stat fields so the caller can check it when they care.

my original suggestion was to add an extra field to struct dirtree for errno.

the patch i just sent out actually just reads errno directly, because
(right now at least) dirtree doesn't corrupt errno.

i also added another test for find, because the ENOENT special case
has a special case: paths provided on the command line are different
from discovered ones (which makes sense, because the former is likely
an error while the latter is likely just a dangling symlink, or
someone removing the same tree at the same time, in which case silence
isn't unreasonable).

> I'm currently using
> st_dev, st_ino, and st_blksize are all zero (which should never happen;
> st_blksize should never be zero but I don't trust it and only one device/ino in
> the system can ever be 0/0 and I think it would be the first file in initramfs?)
>
> Ok, EACCES, ELOOP, ENOENT, ENAMETOOLONG, and sometimes ENOTDIR are _not_ pilot
> error. And the test you added was that ELOOP should NOT get caught. Hmmm...
>
> $ ls -l loop/
> ls: cannot access 'loop/': Too many levels of symbolic links
> $ ./toybox ls -l loop/
> -????????? ? ?    ?    ?                ? loop/
>
> It looks like ELOOP should always get caught as an error?

i think you're confusing ls and find again.

> $ find dirtest
> dirtest
> dirtest/three
> dirtest/dev
> dirtest/one
> dirtest/slink
> find: ‘dirtest/subdir’: Permission denied
> dirtest/subdir
> dirtest/two
> $ ./toybox find dirtest
> dirtest
> dirtest/three
> dirtest/dev
> dirtest/one
> dirtest/slink
> dirtest/two
>
> And that's a behavior difference too. (It complains _and_ shows it, mine's doing
> neither.)

that _is_ a new one on me, but not one i'd seen in practice yet.

> $ ls $(tr \\0 x < /dev/zero | head -c 260)
> ls: cannot access
> 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx':
> File name too long
>
> Not filtered out...
>
> Rob



More information about the Toybox mailing list