Re: [PATCH] find: make pred_empty safer and avoid fd leaks

[James Youngman]

Thanks for this fix.

But, I think the the code I originally I wrote here is broken in a way
that's not completely fixed by your patch.   The case is where the
type of a file changes between directory and non-directory.   The code
as it is now will issue an error whose message corresponds to ENOTDIR.
  If a directory is replaced by an empty file, I think there'a an
argument for returning true without issuing a diagnostic (and hence
without having  find return with a nonzero exit status).

On the assumption that you also think this race should be handled
silently, how about this approach?

Instead of keeping the pre-existing race introduced by the iniail
S_ISDIR check, we could begin by open()ing the file, without
O_DIRECTORY.   If the open fails, issue an error and return.
Otherwise, perform fstat on the fd.   If the fstat fails, issue an
error message and return false.  Otherwise, if the file is a regular
file (S_ISREG, as current code) and it is zero bytes long, return
true.   Otherwise, if it is a non-directory, return false (as now).
Last, handle the directory case.   Perform fdopendir (which, now, we
know cannot fail with ENOTDIR) and attempt readdir on it.   If any
readdir returns non-NULL, return false.

If readdir returns NULL with errno != 0, I think it is probably safest
to issue an error and return false, as now.  Except EOVERFLOW: for
this, we should issue no diagnostic and simply return false (since we
know there is something in the directory, even though it cannot be
correctly represented).   Otherwise (that is, readdir returns NULL
with errno == 0 and we didn't previously get a non-NULL result) return
true.

What do you think?
James.