[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: gnulib broken on systems lacking fchdir

From: Matthew Woehlke
Subject: Re: gnulib broken on systems lacking fchdir
Date: Wed, 29 Nov 2006 17:48:56 -0600
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv: Gecko/20061025 Thunderbird/ Mnenhy/

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:

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?

...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

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?)

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?

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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]