From: Johannes Sixt <j.sixt@viscovery.net>
To: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Cc: Junio C Hamano <gitster@pobox.com>,
GIT Mailing-list <git@vger.kernel.org>,
Marius Storm-Olsen <mstormo@gmail.com>
Subject: Re: [PATCH 4/4] win32: Improve the conditional inclusion of WIN32 API code
Date: Tue, 03 Nov 2009 08:41:17 +0100 [thread overview]
Message-ID: <4AEFDE9D.6060200@viscovery.net> (raw)
In-Reply-To: <4AE746C6.9060409@ramsay1.demon.co.uk>
Ramsay Jones schrieb:
> 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
next prev parent reply other threads:[~2009-11-03 7:41 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-10-27 19:15 [PATCH 4/4] win32: Improve the conditional inclusion of WIN32 API code Ramsay Jones
2009-11-03 7:41 ` Johannes Sixt [this message]
2009-11-04 19:40 ` Ramsay Jones
2009-11-05 7:41 ` Johannes Sixt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4AEFDE9D.6060200@viscovery.net \
--to=j.sixt@viscovery.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=mstormo@gmail.com \
--cc=ramsay@ramsay1.demon.co.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).