Re: new module fchdir

[Jim Meyering]

Bruno Haible <address@hidden> wrote:
> Following Jim's and Paul's ideas for portability of the coreutils to
> BeOS, Woe32 and DJGPP, which all lack an fchdir(), here is a first working
> fchdir module.
>
> The module installs wrappers around open(), close(), opendir(), closedir(),
> dup(), dup2(). Quite heavy; especially the relation to the *-safer modules
> is tricky. Also the 'dup-safer', 'getcwd', 'unistd' modules are affected.
> The many changes to backupfile.c...utime.c are mere optimizations.

Thanks for doing that!
It looks fine to me.

However, one addition that would ease maintenance would be a check to
ensure that the '#undef's (open, close, opendir, closedir) are present
in new files that can benefit from them.  Likewise, something to ensure
that they aren't accidentally added in files where they would cause
a malfunction on systems using the replacement fchdir.  This latter is
especially important, considering that people will test the tools far less
frequently on those fringe systems than on systems that do provide fchdir.

Such checks may well require hard-coded lists of files that would have
to be maintained manually, but even that is better than nothing.  Then,
at least, when adding a new module that uses open, close, opendir, or
closedir, the person adding it will have to consider this issue in order
to get past the check.

For example, a test as naive as this one (print names of files that
appear to call "open", but that lack the #undef) seems to have found
a file (getloadavg.c) that should have the #undef:

  $ t=open; grep -lw "$t (" *.c|xargs grep -L undef.$t'$'
  fd-safer.c
  getloadavg.c
  openat.c

> With these changes, coreutils-6.7 passes "make check" on Linux and MacOS X,
> except for an ENAMETOOLONG error in misc/pwd-long (to be expected) on Linux
> and an rm/deep-1 failure on MacOS X. And on BeOS, it builds as reasonably as
> my previous, more intrusive patch in September 2006.
>
> I put the module under LGPL, so that fts-lgpl can use it. This implies that it
> relies on 'canonicalize-lgpl', not 'canonicalize'.
>
> Opinions?
>
> If you want to test this with coreutils, you need to add fchdir to the
> module list, use --avoid=canonicalize-lgpl, and drop the fchdir-stub.c.

I applied your coreutils changes, too.