[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#9141: fdatasync module proposal
From: |
Bruno Haible |
Subject: |
bug#9141: fdatasync module proposal |
Date: |
Sat, 23 Jul 2011 04:42:23 +0200 |
User-agent: |
KMail/1.13.6 (Linux/2.6.37.6-0.5-desktop; KDE/4.6.0; x86_64; ; ) |
Hi Paul,
> Surely coreutils is not the only program that will have problems
> with fdatasync on Mac OS. How about the following gnulib patches?
> One is for fdatasync, the other for its tests.
Looks mostly good. Just small comments:
> --- a/lib/unistd.in.h
> +++ b/lib/unistd.in.h
> @@ -483,6 +483,34 @@ _GL_WARN_ON_USE (fchownat, "fchownat is not portable - "
> #endif
>
>
> +#if @GNULIB_FDATASYNC@
> +/* Synchronize data changes to a file.
> + Return 0 if successful, otherwise -1 and errno set.
> + See POSIX:2001 specification
> + <http://www.opengroup.org/susv3xsh/fdatasync.html>. */
POSIX:2001 is superseded by POSIX:2008. Even if the wording in both standards
is the same for this function, it is good to encourage people to look at and
to refer to the newest standard. So, here I would write
See POSIX:2008 specification
<http://www.opengroup.org/onlinepubs/9699919799/functions/fdatasync.html>.
*/
> diff --git a/m4/fdatasync.m4 b/m4/fdatasync.m4
> new file mode 100644
> index 0000000..af17970
> --- /dev/null
> +++ b/m4/fdatasync.m4
> @@ -0,0 +1,34 @@
> ...
> + else
> + gl_saved_libs=$LIBS
> + AC_SEARCH_LIBS([fdatasync], [rt posix4],
> + [test "$ac_cv_search_fdatasync" = "none required" ||
> + LIB_FDATASYNC=$ac_cv_search_fdatasync])
> + AC_CHECK_FUNCS([fdatasync])
> + LIBS=$gl_saved_libs
Here one could add a comment like:
dnl Solaris <= 2.6 has fdatasync() in libposix4.
dnl Solaris 7..10 has it in librt.
Just for reference, because in 5 years nobody would remember.
> diff --git a/modules/fdatasync b/modules/fdatasync
> new file mode 100644
> index 0000000..94980ec
> --- /dev/null
> +++ b/modules/fdatasync
> ...
> +fsync [test $HAVE_FDATASYNC = 0]
> +unistd
> +
> +configure.ac:
> +gl_FUNC_FDATASYNC
> +if test $HAVE_FDATASYNC = 0; then
It is safer (w.r.t. future modifications) and more consistent with the
hundreds of other modules to also test $REPLACE_FDATASYNC here:
[test $HAVE_FDATASYNC = 0 || test $REPLACE_FDATASYNC = 1]
> const char *file = "test-fsync.txt";
With this definition, the test-fsync and test-fdatasync programs will write
to the same file. That is, when run with "make -j2", they may collide.
You need to take a different file name for test-fdatasync.
> --- /dev/null
> +++ b/tests/test-fdatasync.c
> @@ -0,0 +1,2 @@
> +#define FSYNC fdatasync
> +#include "test-fsync.c"
> diff --git a/tests/test-fsync.c b/tests/test-fsync.c
> index 2627d0c..6bac01c 100644
> --- a/tests/test-fsync.c
> +++ b/tests/test-fsync.c
> @@ -18,8 +18,12 @@
>
> #include <unistd.h>
>
> +#ifndef FSYNC
> +# define FSYNC fsync
> +#endif
> +
> #include "signature.h"
> -SIGNATURE_CHECK (fsync, int, (int));
> +SIGNATURE_CHECK (FSYNC, int, (int));
>
> #include <errno.h>
> #include <fcntl.h>
> @@ -32,7 +36,7 @@ main (void)
> int fd;
> const char *file = "test-fsync.txt";
>
> - if (fsync (0) != 0)
> + if (FSYNC (0) != 0)
> {
> ASSERT (errno == EINVAL /* POSIX */
> || errno == ENOTSUP /* seen on MacOS X 10.5 */
> @@ -42,7 +46,7 @@ main (void)
> fd = open (file, O_WRONLY|O_CREAT|O_TRUNC, 0644);
> ASSERT (0 <= fd);
> ASSERT (write (fd, "hello", 5) == 5);
> - ASSERT (fsync (fd) == 0);
> + ASSERT (FSYNC (fd) == 0);
> ASSERT (close (fd) == 0);
> ASSERT (unlink (file) == 0);
>
Here, like with test-pselect, I would move everything after the signature test
to a separate file, test-fsync.h, that is included by both test-fsync.c and
test-fdatasync.c. This avoids #ifdefs (following the general rule to prefer
C functions over C macros), and gives freedom to add tests that apply to one
of the functions but not both.
Bruno
--
In memoriam Dmitry Pavlov <http://en.wikipedia.org/wiki/Dmitry_Pavlov_(general)>
- bug#9141: Coreutils Compiler Warnings on OSX 10.7 (Lion), (continued)
- bug#9141: Coreutils Compiler Warnings on OSX 10.7 (Lion), Paul Eggert, 2011/07/21
- bug#9141: [PATCH 1/3] extensions: Enable extensions on MacOS X 10.5 and later., Paul Eggert, 2011/07/22
- bug#9141: [PATCH 1/3] extensions: Enable extensions on MacOS X 10.5 and later., Voelker, Bernhard, 2011/07/25
- bug#9141: [PATCH 1/3] extensions: Enable extensions on MacOS X 10.5 and later., Paul Eggert, 2011/07/25
- bug#9141: [PATCH 1/3] extensions: Enable extensions on MacOS X 10.5 and later., Voelker, Bernhard, 2011/07/25
bug#9141: fdatasync module proposal, Paul Eggert, 2011/07/22
- bug#9141: fdatasync module proposal,
Bruno Haible <=
bug#9140: Coreutils Bug on OSX 10.7 (Lion), Paul Eggert, 2011/07/21
- bug#9140: Coreutils Bug on OSX 10.7 (Lion), Herb Wartens, 2011/07/21
- bug#9140: Coreutils Bug on OSX 10.7 (Lion), Paul Eggert, 2011/07/22
- bug#9140: Coreutils Bug on OSX 10.7 (Lion), Jim Meyering, 2011/07/22
- bug#9140: Coreutils Bug on OSX 10.7 (Lion), Bruno Haible, 2011/07/23
- bug#9140: fsusage: add large volume support on glibc/Hurd, HP-UX 11, Solaris, MacOS X, Bruno Haible, 2011/07/23
- bug#9140: fsusage: add large volume support on glibc/Hurd, HP-UX 11, Solaris, MacOS X, Jim Meyering, 2011/07/23
- bug#9140: fsusage: revert unintended change on AIX, Cygwin, Interix, Bruno Haible, 2011/07/23
- bug#9140: fsusage: revert unintended change on AIX, Cygwin, Interix, Jim Meyering, 2011/07/23
bug#9140: Coreutils Bug on OSX 10.7 (Lion), Jim Meyering, 2011/07/23