Pádraig Brady wrote:
> Take 2 attached.
Take 2 review:
> +#ifdef REPLACE_FALLOCATE
The generated fcntl.h should be maximally standalone. Can you better use
#if @REPLACE_FALLOCATE@ and use an AC_SUBSTed variable REPLACE_FALLOCATE
instead of one defined in config.h?
> +# undef fallocate
Why the undef? By default, we want to see warnings from gcc, and silence
them only when needed. If it is because of fallocate64 and LFS, please add
a comment.
> +#include <sys/types.h>
Includes belong at the top, before the 'extern "C" {'. On some systems,
in C++ mode, you cannot include system header files inside 'extern "C" { }'.
Documentation is missing. Can you add a reference to a well written
specification of this function, or otherwise describe
- what are the arguments?
- what does the function do?
- what is the return value?
> +#include <errno.h>
> +static inline int fallocate (int fd, int mode, off_t offset, off_t len)
If you need extra include files, like <errno.h>, it's a sign that you better
create a file lib/fallocate.c now. You will need that anyway later for the
Solaris special-case implementation.
> + return ENOSYS;
The return value convention described in
http://www.kernel.org/doc/man-pages/online/pages/man2/fallocate.2.html
is a different one. Either this code or that man page is wrong.
> +#endif
> +
> #ifdef __cplusplus
> }
> #endif
You are inserting this piece of code into the section marked
"Declare overridden functions". IMO it would be more logical near the end
of the file, around line 140.
> + gl_cv_func_fallocate=yes, gl_cv_func_fallocate=no)])
In gnulib and coreutils, we've started on 2009-01-14 to improve the m4
macro argument quoting. This means, don't omit [ ] brackets as an
"optimization".
> +Description:
> +Ensure fallocate() is available
This is not a very informative description. How about:
fallocate() function: allocate disk space for a file.
> +errno
This dependency on errno is not needed, I think. See
doc/posix-headers/errno.texi for what the 'errno' module brings.
> +Include:
> +
Here you should note which include file the user should #include
in order to use the functionality.
Bruno
