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

[Christian Schmidbauer]

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.

Hence adding something like this to keep MSVC compatibility:
#if defined(WINAPI) && !defined(_MSC_VER)
 #undef WINAPI
#endif

#ifndef _MSC_VER
 ... the code of the patch above ...
#endif


Or just ignore it and treat MSVC as "non i386/amd64 abi", basically
letting it live inside else
...
#else
 #define WINAPI
#endif

There is still the question if Nine works or makes sense in Windows.