Re: [dev] [sbase] Bugs in find

From: Tavian Barnes <tavianator_AT_tavianator.com>
Date: Thu, 27 Sep 2018 22:56:56 -0400

On Thu, 27 Sep 2018 at 11:06, Evan Gates <evan.gates_AT_gmail.com> wrote:
> On Wed, Sep 26, 2018 at 5:49 PM Tavian Barnes <tavianator_AT_tavianator.com> wrote:
> >
> > Hi! As the author of a find-compatible tool, whenever I find another
> > find implementation I run my testsuite against it to see if I find any
> > bugs in either one. sbase/find helped me identify many places in my
> > POSIX tests that use extensions to POSIX when they shouldn't, so
> > thanks!
>
> Awesome! Those extensions sneak in don't they?

Yeah they sure do.

> [snip]
> Interesting. It looks like we'd have to either cd() or fstatat(). I'm
> thinking we could use dirfd() to get an fd from the DIR* we already have.
> Then we could pass the fd along in order to use fstatat(). If not for the
> -path primary we could just pass the fd and the filename when recursing.
> But because of -path it would probably make the most sense to pass the
> fd, filename, and full path.

fstatat() is better than chdir() IMO. Can't forget to chdir() back in
-exec for example. Your coding style says you use POSIX 2008 so we
can rely on it. There's additional complications if you want to
handle directory structures deeper than OPEN_MAX.

Any objections to changing find.c to use recurse()? That way this
could be fixed for a few tools at once.

> > Despite that, it exits successfully, which is probably also a bug.
>
> Good point. I wanted to keep on processing instead of exiting
> immediately, but it makes sense to set a flag so we exit failure.

Makes sense!

> [snip]
> > foo/baz is not newer than itself though.
>
> Well that's just weird. In a brief look at the code I'm not sure why
> that's happening.

Looks like stat() instead of lstat() in get_newer_arg(). A strict
reading of the POSIX spec implies it should always be lstat() (the
-H/-L docs speak only about the path operands, not other arguments
that happen to be paths), but I don't know of any find implementations
that do that. GNU find and several others respect -H/-L for whether
to follow symbolic links passed to -newer. But the POSIX conformance
tests check for this.

> > - find -perm doesn't support X -
> >
> > $ ./find . -perm a+rX,u+w
> > ./find: a+rX,u+w: invalid mode
>
> That seems to be missing from libutil/mode.c

I see. So this is missing from sbase/chmod too I guess. Guess I can
fix both at once :)

> [snip]
> Always happy to see someone excited about sbase. Thanks for reporting
> the bugs. Would you be interested in submitting patches, too? That would
> be most helpful. You can think of it as an excuse to see the inner
> workings of another find implementation :-)

Yep, I'll start digging into it.

-- 
Tavian Barnes
Received on Fri Sep 28 2018 - 04:56:56 CEST

This archive was generated by hypermail 2.3.0 : Fri Sep 28 2018 - 05:00:08 CEST