Re: gnulib broken on systems lacking fchdir

[Matthew Woehlke]

Eric Blake wrote:
> Matthew Woehlke writes:
> > > A well-written fchdir module would be welcome.  Such a module would have
> > > no effect on coreutils proper, other gnulib modules, or any system with
> > > fchdir support.
> > Ok, thanks... Anyway, here's a start:
> >
> > [snip]
> > int
> > open (const char* pathname, int flags)
>
> You need to remember the optional mode argument to open.

I saw that in the manpage, but didn't get it. Last I checked C does not 
support function overloading? So I am not sure how to declare both.

> > {
> >    int fd = SYS_open (pathname);
> >    if (fd >= 0)
> >      fdheap_add (fd, pathname);
>
> Make sure fdheap_add duplicates the name; if you go with xstrdup, then your 
> replacement should be GPL (since LGPL implies library code, but libraries 
> shouldn't arbitrarily die on memory allocation failures).  Also, GNU coding 
> standards recommend using just 'name', rather than 'filename' or even 'file'.

Well, I stole what glibc's manpage called the arg. :-) I'm 
half-expecting that someone will want to standards-sanitize any code I 
submit, which is fine because I know I don't know the standards very 
well. :-)

The licensing issue is a more important question; do any of the fchdir 
users need to stay LGPL?

> > [snip]
> > ...but as I said, I'm not really familiar with how one needs to overload 
> > open()/close(), or if fopen()/fclose() and who-knows-what-else also need 
> > to be overridden. 
>
> Actually, gnulib already has an example of overloading open, via the *-safer 
> modules.  The idea is that you arrange for <config.h> to #define open rpl_open, 
> then all your code that directly calls open (except for the one .c file where 
> you actually implement rpl_open) will get your replacement.  But you are 
> correct that existing library code that already calls open under the hood (such 
> as fopen) will not go through your wrapper, so you have to decide in advance 
> how many functions need to be wrapped.

Ok, that's the style I am familiar with, but as you say, then I will 
have to audit where fchdir() fd's are coming from, and unfortunately it 
makes maintenance ickier.

> I think a first thing to do would be to see where fchdir is used in the fts 
> implementation.  If we can guarantee that fts always uses fchdir on fds that it 
> obtained by open and/or opendir/dirfd from within fts, rather than on random 
> fds from external sources, then that is all the more you would have to wrap 
> (calling fchdir(fileno(fopen(...))) would then need to fail; and I would try to 
> make the replacement fchdir smart enough to distinguish EBADF for non-open fd 
> vs. EOPNOTSUPP for open fd not found in the fd-to-name map).  In other words, 
> you don't have to replace ALL uses of open, only the uses that are likely to be 
> a target of fchdir.

As per the above comment, I don't think that is going to work, since 
anyone editing the code then needs to know to use the fchdir-replacement 
functions, which is of course a maintenance nightmare. But now I wonder 
if only replacing opendir() is enough? Will have to check. :-)

> > What about the FD table; should it be a hash table, a binary tree, an 
> > ordered linked list, or something else entirely?
>
> Gnulib already provides the gl_list module. 
> [snip]
> Once you use the gl_list API, it is only a one-line code change and
> reinvocation of gnulib-tool to pick up the new underlying list
> implementation.

Hmm, something I know absolutely nothing about, but at least it sounds 
like something I can use safely ("safe" in the legal sense).

> > (Can autotools make compilation of this dependent on HAVE_FCHDIR so that 
> > it doesn't need to be in One Big Preprocessor Condition? Am I guessing 
> > right that this - with overloading open()/close() correctly - would mean 
> > that no header is needed?)
>
> [snip]
> In short, all the magic for deciding and providing the replacement is in 
> the .m4 file at configure time; every other module that uses fchdir has to pick 
> up a new "fchdir.h" header, but can otherwise assume POSIX semantics with no 
> further changes.

...except that a declaration for fchdir() is already provided (hmm, 
sorry, should have mentioned it is the link that dies, not the compile). 
So why would an fchdir.h be needed?

--
Matthew
Emacs is a nice OS - but it lacks a good text editor.
That's why I am using Vim.  -- Anonymous