Build failure on musl based system (incorrect fmemopen check)

[deepcube]

I'm running a distro that uses musl libc instead of glibc. The build fails because musl does support fmemopen, but the ifdef checks are false. I was able to build easily by adding -D_POSIX_C_SOURCE=200809L, but that shouldn't be necessary.

fmemopen is a standard POSIX interface[0] and should be available without any feature test macros. The fact that it isn't available on MacOS makes that the exception. The comment says "Assume BSD without native fmemopen," but the BSDs do have fmemopen[1][2][3]. I think that the ifdef should only be

#if defined(__APPLE)

and should not include anything about _GNU_SOURCE or _POSIX_C_SOURCE. An even better solution would be a configure check to see if fmemopen is available, as this would still work correctly if Apple does add it in the future. However I do understand the pragmatism of just checking for Apple and moving on for now instead of adding an extra configure step.

I'm happy to submit either patch once an agreement is reached as to the correct direction.

[0] https://pubs.opengroup.org/onlinepubs/9699919799/functions/fmemopen.html [1] https://man.netbsd.org/fmemopen.3 [2] https://man.openbsd.org/fmemopen.3 [3] https://www.freebsd.org/cgi/man.cgi?query=fmemopen

[timg236]

Updating the define sounds reasonable.

Realistically, we aren't going to build or test this on anything other that Raspberry Pi OS (Debian) and Windows Cygwin. However, if people want to add simple patches (with suitable comments to avoid bitrot!) for other platforms then that's fine.

[deepcube]

@timg236 OK, thanks.

@jrmithdobbs @pelwell Can either of you chime in about

45c7237 (Fixup for recent firmware inclusion changes (#34)) 17f6b01 (Fix cross-platform building)

I want to better understand your motivations with the ifdef checks and make sure that any patch I submit doesn't break your use cases.

[pelwell]

From https://man7.org/linux/man-pages/man3/fmemopen.3.html:

CONFORMING TO

       POSIX.1-2008.  This function is not specified in POSIX.1-2001,
       and is not widely available on other systems.

       POSIX.1-2008 specifies that 'b' in mode shall be ignored.
       However, Technical Corrigendum 1 adjusts the standard to allow
       implementation-specific treatment for this case, thus permitting
       the glibc treatment of 'b'.

So although it is a POSIX function, it isn't in all versions of POSIX, hence the version check.

[deepcube]

Well this just keeps getting more interesting...

It seems OS X has supported fmemopen since version 10.13 released in 2017[0]. If you are OK dropping support for older versions of OS X we can just drop the check and the fallback fmemopen completely. Are people still using OS X that old?

The problem with the current check is that the libc implementation doesn't define _POSIX_C_SOURCE to tell the user which features are available. Instead the user is supposed to define _POSIX_C_SOURCE to control which features the implementation makes available[0]:

Feature test macros allow the programmer to control the
definitions that are exposed by system header files when a
program is compiled.

Thanks to a pointer from dalias in #musl, POSIX C libraries do define _POSIX_VERSION in unistd.h[2]. If OS X/MacOS properly define that, then we can use it to decide whether or not to include the fallback fmemopen.

@pelwell @jrmithdobbs Do either you currently use an old enough OSX that it doesn't support fmemopen (older than 10.13)? If so can you run a quick test to see what _POSIX_VERSION is set to?

#include <stdio.h>
#include <unistd.h>
int main(void) {
    printf("%ld\n", _POSIX_VERSION);
    return 0;
}

[0] https://news.ycombinator.com/item?id=25968777 [1] https://man7.org/linux/man-pages/man7/feature_test_macros.7.html [2] https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/unistd.h.html

[pelwell]

On 10.13.6 I get 200112, which correctly causes fmemopen.c to be loaded.

[deepcube]

I've created two commits.

One adds a simple configure script: https://github.com/deepcube/usbboot/commit/692dad174477502f584272a56f4370272ceba238

The other just changes the logic to check _POSIX_VERSION: https://github.com/deepcube/usbboot/commit/6aa770a948f89d222064f8d0223d4ede7a5f5003

The first is more correct, will work on more systems, and may be a useful starting point if other things need to be checked in the future. But it is also more complicated.

@timg236 If you have a strong preference or comments let me know. I can open a pull request as well.

@pelwell If you have the time to test I would appreciate it. I don't have a mac or BSD system around.

[timg236]

I'd rather not switch to using configure

[pelwell]

Agreed. I'm happy with the second option - https://github.com/deepcube/usbboot/commit/6aa770a948f89d222064f8d0223d4ede7a5f5003 - since an absence of the _POSIX_VERSION macro also triggers the fallback to fmemopen.c.

[pelwell]

(Auto-closed because of the Fixes: tag in https://github.com/raspberrypi/usbboot/pull/133)