[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: rm (remove.c): Rewrite to use fts: request for review
From: |
Eric Blake |
Subject: |
Re: rm (remove.c): Rewrite to use fts: request for review |
Date: |
Fri, 28 Aug 2009 21:40:17 +0000 (UTC) |
User-agent: |
Loom/3.14 (http://gmane.org/) |
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