Linking against libnfs fails with musl C library

[joerg-krause]

The musl C library is very strict when it comes to POSIX. As libnfs.h uses struct timeval, which shall be defined in <sys/time.h> (as defined by POSIX), linking any binary or other library with libnfs fails on a musl based system:

$SYSROOT/usr/include/nfsc/libnfs.h:965:23: error: field 'atime' has incomplete type 'timeval'
        struct timeval atime;

Is there any reason to not unconditionally include <sys/time.h> for all POSIX systems?

[sahlberg]

On Mon, Nov 5, 2018 at 10:27 AM Jörg Krause [email protected] wrote:

The musl C library is very strict when it comes to POSIX. As libnfs.h uses struct timeval, which shall be defined in <sys/time.h> (as defined by POSIX http://pubs.opengroup.org/onlinepubs/009604499/basedefs/sys/time.h.html), linking any binary or other library with libnfs fails on a musl based system:

$SYSROOT/usr/include/nfsc/libnfs.h:965:23: error: field 'atime' has incomplete type 'timeval' struct timeval atime;

Is there any reason to not unconditionally include <sys/time.h> for all POSIX systems?

Windows does not have sys/time.h for example, which is why we need a conditional for its inclusion in libnfs.h :

#if defined(ANDROID) || defined(AROS)
|| ( defined(APPLE) && defined(MACH) ) #include <sys/time.h>

Does the musl environment provide a define or symbol that we can add to the list above?

—

You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/sahlberg/libnfs/issues/272, or mute the thread https://github.com/notifications/unsubscribe-auth/AAeNkDxihTW-42DY4TMqoZufKOce1gu8ks5ur4XfgaJpZM4YNmT5 .

[joerg-krause]

No, musl does not have such symbol or define by design. Does Windows have own? If so, why not check for this one?

[Nable80]

Windows does not have sys/time.h for example

But it's not a POSIX system. Btw, it's not a new problem and the usual solution is to detect win32 environment and include either time.h or winsock2.h[1], you can find an example in [2].

[1] https://docs.microsoft.com/en-us/windows/desktop/api/winsock/ns-winsock-timeval [2] https://trac.osgeo.org/mapserver/ticket/602#comment:6

[sahlberg]

On Mon, Nov 5, 2018 at 5:26 PM Nable80 [email protected] wrote:

Windows does not have sys/time.h for example

But it's not a POSIX system. Btw, it's not a new problem and the usual solution is to detect win32 environment and include either time.h or winsock2.h[1], you can find an example in [2].

[1] https://docs.microsoft.com/en-us/windows/desktop/api/winsock/ns-winsock-timeval [2] https://trac.osgeo.org/mapserver/ticket/602#comment:6

Thanks for this info (I am mostly windows ignorant) This looks like a very simple solution and it sounds like it will work. Can you send me a pull request so you get credit for the fix?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/sahlberg/libnfs/issues/272#issuecomment-435777857, or mute the thread https://github.com/notifications/unsubscribe-auth/AAeNkH3No86olzv5UHYsHX9JS8dV1GSYks5ur-ghgaJpZM4YNmT5 .

[Nable80]

Unfortunately I'm mostly windows-ignorant too.

OK. As far as I understand in order to prepare the pull request I should test that this fix doesn't break anything on windows platforms (both cygwin and native builds) AND fixes the problem with musl AND doesn't break any non-windows builds. Are there any supported non-windows platforms where sys/time.h shouldn't be included?