Re: rm (remove.c): Rewrite to use fts: request for review

[Eric Blake]

Jim Meyering <jim <at> meyering.net> writes:

> It'd be great if another pair of eyes could glance through
> these changes (diffs look "big", but most hunks are simply removals).

I haven't looked at this closely yet, but one thing raised a question in my 
mind: with the switch to fts, can 'rm -ri dir <&-' ever see fd 0 tied to an 
open directory when querying, or does the first query still occur before any fd 
is created?  If the former, that lends more urgency to my goal of implementing 
opendir_safer.

In looking at gnulib, I noticed that fdopendir is hidden inside openat.c, and 
is rather inefficient on mingw when the fchdir module is also in use (since the 
fchdir module maintains a name/fd mapping, we could just refer to that for the 
proper opendir() invocation rather than wasting save_cwd/fchdir/opendir
(".")/restore_cwd.  I guess my first course of action will be moving fdopendir 
into its own module, along with creating unit tests for fdopendir and fchdir.

Also, mingw does not provide dirfd and its struct DIR does not have room for an 
fd, so gnulib's dirfd replacement currently just returns ENOTSUP (as permitted 
by POSIX).  One option is to keep things like this, keeping openat's 
implementation of closing an fd up front during fdopendir.  But another option 
would be modifying the fchdir wrapper to also save an open handle to any 
directory created by opendir or fdopendir, and close it during closedir; this 
would also help the semantics of the fchdir.c file which asks that open 
directories not be renamed in the meantime (mingw already has the property that 
you can't rename an open directory, but since we currently don't maintain an 
open handle, we can't enforce the current assumption in fchdir).  Meanwhile, 
the mingw definition of struct DIR does include an embedded name of the 
directory being visited, so dirfd could be implemented to open an fd handle 
rather than failing with ENOTSUP.

At any rate, if dirfd gives a non-negative result, then opendir_safer must swap 
out the underlying fd; but if dirfd fails with -1, then there is no fd to 
protect in the first place.  Therefore, I'm assuming that it is better to 
implement opendir_safer like this (plus better error handling):

DIR *result = opendir (name);
int fd = dirfd (result);
if (0 <= fd && fd <= 2)
  {
#if !HAVE_FDOPENDIR
    /* With no fdopendir implementation, dirfd should always return
       -1, so we should never get here.  */
    abort ();
#else
    DIR *d = fdopendir (dup_safer (fd));
    closedir (result);
    result = d;
#endif
  }
return result;

rather than this:

int fd = open (name, O_DIRECTORY | O_CLOEXEC | O_RDONLY);
return fdopendir (dup_safer (fd));

so that opendir_safer can be made to work on mingw even if we aren't using the 
gnulib fchdir/fdopendir modules.

-- 
Eric Blake