On Thu, 28 Jan 2010, Otto Moerbeek wrote:
> On Thu, Jan 28, 2010 at 03:25:21AM -0500, Daniel Dickman wrote:
>
> > Note: the patch below is NOT recommended for 4.7 as this will change
> > the semantics of fts_open and will BREAK things like "gzip -" and
> > possibly other stuff.
> >
> > Calls to fts_open should probably be audited first...
>
> yes, this is tricky stuff. Compare this to getting a Permission
> denied, because the dir is not accessible. Currently find also returns
> with 0 in that case.
Yes. I think this is also a bug.
>
> The main question is: should toplevel dirs be handled differently than
> deeper dirs?
I think so. POSIX says:
The following exit values shall be returned:
0
All path operands were traversed successfully.
>0
An error occurred.
So the error condition above should be logically the same as the
following:
>0
Some path operand was not traversed successfully.
And since the deeper dirs are not "path operands" I think the implication
is that being able to traverse them or not shouldn't matter.
>
> -Otto
>
> >
> >
> > On Thu, Jan 28, 2010 at 2:06 AM, Daniel Dickman <[email protected]> wrote:
> > > As reported on the git mailing list[1], find(1) should exit with non-zero
> > > status when a non-existent path is given which I think is also the intent
> > > of the find(1) code.
> > >
> > > [1] http://marc.info/?l=git&m=126186443524638
> > >
> > > For example:
> > >
> > > B B B B # find /no-such-path
> > > B B B B find: fts_open: No such file or directory
> > > B B B B # echo $?
> > > B B B B 0
> > > B B B B ^^^ <--- should be something non-zero.
> > >
> > > Looking at the code for find, we have the following in
> > > /usr/src/usr.bin/find/find.c, line 153:
> > >
> > > B B B B if (!(tree = fts_open(paths, ftsoptions, NULL)))
> > > B B B B B B B B err(1, "fts_open");
> > >
> > > This looks right and fts_open(1) says:
> > >
> > > B B B B If an error occurs, fts_open() returns NULL and sets
> > > B B B B errno appropriately.
> > >
> > > Debugging, it seems like errno comes back as ENOENT but the variable
> > > "tree" is not set to null...
> > >
> > > Debugging a bit more it seems like "fts_open" calls "fts_stat" which
> > > returns FTS_NS but the code path for fts_open doesn't go into the error
> > > path at this point. Therefore, I propose the patch below.
> > >
> > > With the patch applied I get:
> > >
> > > B B B B # find /no-such-path
> > > B B B B find: fts_open: No such file or directory
> > > B B B B # echo $?
> > > B B B B 1
> > >
> > > Index: fts.c
> > > ===================================================================
> > > RCS file: /usr/cvs/src/lib/libc/gen/fts.c,v
> > > retrieving revision 1.43
> > > diff -u -r1.43 fts.c
> > > --- fts.c B B B 27 Aug 2009 16:19:27 -0000 B B B 1.43
> > > +++ fts.c B B B 28 Jan 2010 06:48:11 -0000
> > > @@ -117,6 +117,10 @@
> > > B B B B B B B B p->fts_accpath = p->fts_name;
> > > B B B B B B B B p->fts_info = fts_stat(sp, p,
> > > ISSET(FTS_COMFOLLOW));
> > >
> > > + B B B B B B B /* stat(2) failed, so fts_open should error. */
> > > + B B B B B B B if (p->fts_info == FTS_NS)
> > > + B B B B B B B B B B B goto mem3;
> > > +
> > > B B B B B B B B /* Command-line "." and ".." are real directories.
> > */
> > > B B B B B B B B if (p->fts_info == FTS_DOT)
> > > B B B B B B B B B B B B p->fts_info = FTS_D;
