Hello Pádraig,
The documentation files you sent document the status before any 'fallocate'
module is added to gnulib. So I committed them for you:
2009-05-21 Pádraig Brady <p...@draigbrady.com>
* doc/glibc-functions/fallocate.texi: New file.
* doc/gnulib.texi: Include it.
The fallocate.texi will need a modification that goes along with the
'fallocate' module.
> This is my first gnulib module
Ok, here are detail comments:
- fallocate.c should have a GPLv3 copyright header.
- A rpl_fallocate function that always returns ENOSYS makes no sense for gnulib.
In gnulib, we enable a function when we have working code for it. If the
replacement can be implemented only on some platforms, we make sure the .h
file
defines an indicator that users can test with #if or #ifdef.
In coreutils, the policy is different: coreutils at some places has dummy
stubs, and is willing to compile in function calls to these dummy stubs on
platforms that lack the particular functionality. Other projects that use
gnulib prefer to use a #if around the code that uses the functionality.
- In fallocate.c, the "#undef fallocate" should go away. It makes it impossible
to make a namespace-clean library by adding a
#define fallocate libfoo_private_fallocate
to config.h.
- glibc declares fallocate() in <fcntl.h>. Therefore gnulib should do the same.
There is no use in creating a file "fallocate.h"; instead, put the
declarations
into fcntl.in.h.
- In fallocate.m4 you are doing
AC_DEFINE([fallocate], [rpl_fallocate], [replacement stub])
This define is better done in the .h file (fcntl.in.h in this case).
Otherwise you get problems when a platform has an 'fallocate' function.
We used that mix of config.h and *.in.h idiom in the beginning, but it
turned out to be more maintainable to put the entire replacement code
into the *.in.h file.
- In fallocate.m4 the invocation of AC_TRY_LINK takes some time (it runs
the compiler and linker) and therefore should be protected by a cache
variable. AC_CACHE_CHECK is your friend.
> Note also "fallocate()" functionality is available on
> solaris using the fcntl(fd, F_ALLOCSP, ...) interface.
> Hopefully this can be wrapped by this fallocate() at
> some stage.
Yes, sure, please do this. F_ALLOCSP and F_ALLOCSP64. The more platforms
a replacement supports, the better.
> Note fallocate() is unfortunately not available on 32 bit
> fedora 11 at least when AC_SYS_LARGEFILE is used due to:
> https://bugzilla.redhat.com/show_bug.cgi?id=500487
IMO this is not worth working around, because it's likely to be fixed
quickly in glibc.
Bruno
