On 07/04/2013 11:21 PM, Diego Biurrun wrote:
> This was pushed less than two hours after being posted.  Can we *please*
> not rush things so much?  A broken AIX fate box is hardly an urgency
> that needs to get fixed ASAP...

Had been discussed for more than it was worth from start.

> This is not the first, but more like the 100th time I complain about
> rushed pushing of patches.  I propose a 24h embargo on anything that is
> not a fix for really urgent breakage.

Apparently Sean was in hurry or so it looked to me.

> The noun is "workaround", the verb is "to work around"; AIX-specific.
> Then "class usage" is confusing, we do not use classes, there are
> identifiers named "class".

Patch welcome, as I said for yet too many times that such review should
be provided as an amended patch.

> Work around class() function in AIX math.h clashing with identifiers
> named "class".
> 
> Also, this is missing a license header.

This file isn't copyrightable to begin with. If some people deem it
copyrightable I'll add the usual MIT license boilerplate.

>> +#ifndef COMPAT_AIX_MATH_H
>> +#define COMPAT_AIX_MATH_H
> 
> COMPAT is not a valid prefix, it should be LIBAV or LIBAV_COMPAT.

As stated above.

>> +#define class class_in_math_h_causes_problems
>> +
>> +#include_next <math.h>
>> +
>> +#undef class
>> +
>> +#endif /* COMPAT_AIX_MATH_H */
>> --- a/configure
>> +++ b/configure
>> @@ -3049,6 +3049,9 @@ enabled spic && enable_weak pic
>>
>>   # OS specific
>>   case $target_os in
>> +    aix)
>> +        add_cppflags '-I\$(SRC_PATH)/compat/aix'
>> +        ;;
> 
> math.h is a libc header.  Detecting the AIX libc seems cleaner.

Sure, can be done but won't make any difference and the check as it is
is much simpler (and I expect more headers hackery will appear on AIX
since who is maintaining that systems has quite original ideas).

lu

_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to