a-j-wood/pv

Mac OS X: implicit declaration of function 'fdatasync' is invalid

Closed this issue · 3 comments

TurtleWilly commented

Building the latest pv 1.7.24 I encounter this "implicit declaration of function 'fdatasync' is invalid" issue. I think this was introduced by the new --sync option implementation:

…
gcc -O -I./src/include -Isrc/include -DHAVE_CONFIG_H -DLOCALEDIR=\"/usr/local/share/locale\" -c -o src/pv/string.o src/pv/string.c
gcc -O -I./src/include -Isrc/include -DHAVE_CONFIG_H -DLOCALEDIR=\"/usr/local/share/locale\" -c -o src/pv/transfer.o src/pv/transfer.c
src/pv/transfer.c:130:9: warning: implicit declaration of function 'fdatasync' is invalid in C99 [-Wimplicit-function-declaration]
                        if ((fdatasync(fd) < 0) && (EIO == errno)) {
                             ^
1 warning generated.
gcc -O -I./src/include -Isrc/include -DHAVE_CONFIG_H -DLOCALEDIR=\"/usr/local/share/locale\" -c -o src/pv/watchpid.o src/pv/watchpid.c
ld -r -o src/pv.o  src/pv/cursor.o src/pv/display.o src/pv/file.o src/pv/loop.o src/pv/number.o src/pv/signal.o src/pv/state.o src/pv/string.o src/pv/transfer.o src/pv/watchpid.o
sh ./autoconf/scripts/po2table.sh ./src/nls/de.po ./src/nls/fr.po ./src/nls/pl.po ./src/nls/pt.po > src/nls/table.c
…

This is a really strange one. It warns, but still links fine. But Mac OS X doesn't have this function at all in the SDK includes. There's some syscall by the same name, but with an entirely different prototype. I guess that gets magically linked up. But it wouldn't give us an expected result.

HAVE_FDATASYNC should be forced to be always undefined for Mac OS X. I don't think there's an easy way to properly detect it in the configure script (as it warns, but doesn't fail).

As workaround I now pass ac_cv_func_fdatasync=no as argument to configure to void out the test and keep HAVE_FDATASYNC undefined.

Some more (hopefully helpful) information about this issue:
gbrault/picoc#145
axboe/fio#835
php/php-src#6882

As I understand it: pv's fdatasync call basically will now link to this: https://github.com/apple/darwin-xnu/blob/d4061fb0260b3ed486147341b72468f836ed6c8f/bsd/vfs/vfs_syscalls.c#L7708 which is an entirely different function from what pv is expecting to call.

a-j-wood commented

Thanks for the links. The commentary and links suggest that behind the scenes, calling that function will kind of do what pv expects anyway (so the warning is probably harmless), but there is a note in the function's manual about how to check whether fdatasync() is supported by looking at _POSIX_SYNCHRONIZED_IO, so I've committed a change which might help.

Can you please try a "git clone https://github.com/a-j-wood/pv.git" followed by a "./generate.sh && ./configure && make" and see if this has cleared it up?

  • 👍1
TurtleWilly commented

Looks like it works. 👍

It still seems to positively detect it during the configure process:

…
checking for fdatasync... yes
…

but is properly not included during the compilation, so no suspicious warning is generated:

…
gcc -O -I./src/include -Isrc/include -DHAVE_CONFIG_H -DLOCALEDIR=\"/usr/local/share/locale\" -c -o src/pv/string.o src/pv/string.c
gcc -O -I./src/include -Isrc/include -DHAVE_CONFIG_H -DLOCALEDIR=\"/usr/local/share/locale\" -c -o src/pv/transfer.o src/pv/transfer.c
gcc -O -I./src/include -Isrc/include -DHAVE_CONFIG_H -DLOCALEDIR=\"/usr/local/share/locale\" -c -o src/pv/watchpid.o src/pv/watchpid.c
…

For good measurement and verification I added a:

#ifdef __APPLE__
# error fdatasync is problematic!	
#endif

into the respective code block in transfer.c and it wasn't triggered. So all is good and the new "_POSIX_SYNCHRONIZED_IO" handling seems to do its job.

To further verify things I also grepped my Mac OS X SDK includes and a _POSIX_SYNCHRONIZED_IO definition was found in $SDKPATH/usr/include/unistd.h and it is defined to -1 like this #define _POSIX_SYNCHRONIZED_IO (-1) /* [SIO] */.

a-j-wood commented

Thanks for the detailed confirmation - I will close this issue.

  • 👍1