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