Re: [PATCH 4/4] win32: Improve the conditional inclusion of WIN32 API code

From: Johannes Sixt <j.sixt@viscovery.net>
Date: Nov 3, 2009, 07:41 UTC
Message-ID: <4AEFDE9D.6060200@viscovery.net>
In-Reply-To: <4AE746C6.9060409@ramsay1.demon.co.uk>

Ramsay Jones schrieb:

Show 10 quoted lines

> After experimenting with several win32 compilers, it is clear
> that the _WIN32 macro should be used to guard the inclusion
> of the main WIN32 API header files, particularly in the main
> git-compat-util.h header. In addition, the cygwin build should
> restrict the use of the WIN32 API to compat/cygwin.c
> 
> In order to avoid mistakes with the use of WIN32 and _WIN32
> macros, define a new WIN32_API macro, with a single point of
> definition in git-compat-util.h, to isolate WIN32 specific
> code.

IMO, the name "WIN32_API" is very misleading. It suggests that it can be used to protect sections of code that use the API. But in fact all places outside (and even some places in) the compatibility layer use "WIN32" to protect *platform* specific code (as opposed to the API specific code).

In this light, the name "WIN32" isn't to the point, either, but it does at least not pretend that the code is about API.

Therefore, I think that a better change would be to treat _WIN32 and WIN32 as synonyms, and to make sure that both are defined if at least one of them is defined.

It may be that I understand something incorrectly; but then I blame the justification that you gave. In this case, it would be helpful to reword the commit message, and perhaps add some results from your experiments.

-- Hannes