[FFmpeg-devel] [PATCH v2 1/1] configure: Refactor setting of feature test macro _XOPEN_SOURCE

[Jörg Krause]

On 09/18/2014 05:26 PM, Michael Niedermayer wrote:
> On Thu, Sep 18, 2014 at 04:22:27PM +0200, Jörg Krause wrote:
>> glibc, uclibc, and musl uses feature test macros to expose definitions
>> conforming to the standards ISO C, POSIX and extensions. According to
>> which feature test macros are defined by the user or the compiler,
>> the header file <features.h> used by these libraries internally defines
>> various other macros.
>>
>> glibc and uclibc also defines release test macros, eg __GLIBC__ and
>> __UCLIBC__ in <features.h>. musl does not have (and do not want) such
>> a macro like __MUSL__.
>>
>> Building ffmpeg with the musl toolchain needs the feature test macro
>> _XOPEN_SOURCE=600 to be defined. As it is not possible to detect musl
>> check for the <features.h> header file, which is assumed to be specific
>> to glibc, uclibc, and musl.
>>
>> Signed-off-by: Jörg Krause <jkrause at posteo.de>
>> ---
>> Changes  v1 -> v2:
>>    - do not set _XOPEN_SOURCE twice for glibc and uclibc
> why do you not add the musl check at the end of the if/elif chain as
> suggested ?
> does that have some issue or problem ?

The idea was to group the C libraries which support the feature test 
macros and set the shared definitions at one place.

> now your patch removes POSIX_C_SOURCE=200112 for glibc & uclibc
> maybe thats ok, maybe its not but it certainly doesnt belong in a
> patch that adds musl support
> If you think this change is desireable, please submit a seperate
> patch

I see.

> also seperating the existing checks between a group which has
> features.h and one that does not, would need someone to check if thats
> correct for each
> if any thats in the "no features.h" group has a header with the name
> features.h then it would break
>
> and the patch also breaks mingw cross compile
>
> i get
> -HOSTCPPFLAGS= -D_ISOC99_SOURCE -D_POSIX_C_SOURCE=200112 -D_XOPEN_SOURCE=600
> +HOSTCPPFLAGS= -D_ISOC99_SOURCE

Okay, maybe better not touch the existing checks. I will revert this...

>
>> +        elif check_${pfx}cpp_condition features.h "defined __GLIBC__"; then
>> +            eval ${pfx}libc_type=glibc
>> +    fi
> the "fi" is misleadingly indented or rather not indented

Not properly intended.