[Mesa-dev] [PATCH] st/nine: correctly fold guards around define WINAPI

[Grazvydas Ignotas]

On Fri, Apr 15, 2016 at 9:46 PM, Christian Schmidbauer
<ch.schmidbauer at gmail.com> wrote:
> On 15/04/16 17:06, Emil Velikov wrote:
>> From: Emil Velikov <emil.velikov at collabora.com>
>>
>> The __i386__ and __x86-64__ macros are gcc/clang specific, thus one does
>> not need the __GNUC__ at the top.
>>
>> Additionally, having _M_IX86 and _M_X64 in the same block (and even use
>> __attribute__(foo)) is wrong as those are set by MSVC.
>>
>> If at some point we do start building with the Sun/Oracle compiler we
>> might need to add the __i386 and __x86-64 and explicit checks for
>> __attribute__(foo) as the latter is not something that exists there.
>>
>> Cc: Christian Schmidbauer <ch.schmidbauer at gmail.com>
>> Cc: Axel Davy <axel.davy at ens.fr>
>> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
>> ---
>>  include/D3D9/d3d9types.h | 16 +++++++---------
>>  1 file changed, 7 insertions(+), 9 deletions(-)
>>
>> diff --git a/include/D3D9/d3d9types.h b/include/D3D9/d3d9types.h
>> index dce5533..bcce061 100644
>> --- a/include/D3D9/d3d9types.h
>> +++ b/include/D3D9/d3d9types.h
>> @@ -178,15 +178,13 @@ typedef struct _RGNDATA {
>>  #undef WINAPI
>>  #endif /* WINAPI*/
>>
>> -#ifdef __GNUC__
>> -  #if (defined(__x86_64__) && !defined(__ILP32__)) || defined(_M_X64)
>> -    #define WINAPI __attribute__((ms_abi))
>> -  #elif defined(__i386) || defined(_M_IX86)
>> -    #define WINAPI __attribute__((__stdcall__))
>> -  #else /* neither amd64 nor i386 */
>> -    #define WINAPI
>> -  #endif
>> -#else /* __GNUC__ */
>> +#if defined(__x86_64__) && !defined(__ILP32__)
>> +  #define WINAPI __attribute__((ms_abi))
>> +#elif defined(__i386__)
>> +  #define WINAPI __attribute__((__stdcall__))
>> +#elif defined(_M_IX86) /* MSVC specific, do we even use it ? */
>> +  #define WINAPI
>> +#elif defined(_M_X64) /* MSVC specific, do we even use it ? */
>>    #define WINAPI
>>  #endif
>>
>
> You forgot the last "#else ... #define WINAPI", which is the important
> part/build fix on other abis than i386 and amd64.
> Apart from that, removing GNUC (as only gcc, clang and MSVC are
> supported by Mesa) looks fine to me. The original intention was to
> keep the patch as compatible with as many compilers as I could think
> of, hence the second guard.
>
> About the MSVC bits:
> I can think only of two scenarios for having a WINAPI define beforehand:
>
> 1) Some other code needs to be compatible with Windows. Silently
> undefining and re-defining WINAPI in such a case might break their
> code, but Nine (and hence Wine) will run properly. I guess this was
> the intention of the original author.
>
> 2) You are compiling Nine with MSVC. MSVC already defines the proper
> WINAPI, so I assume you don't want to remove WINAPI in such a case.

That's not entirely accurate, WINAPI is defined by platform headers,
not the compiler itself, so _MSC_VER checking would not be correct.
But yeah if something defined WINAPI already, it might be reasonable
to assume they know what they're doing and leave it alone.

Gražvydas