Re: [boost] [pool] detail/mutex.hpp may include windows.h and pollute namespace with windows definitions.

Subject: Re: [boost] [pool] detail/mutex.hpp may include windows.h and pollute namespace with windows definitions.
From: Marsh Ray (marsh_at_[hidden])
Date: 2010-10-11 14:18:57

On 10/11/2010 06:15 AM, Stewart, Robert wrote:
> Marsh Ray wrote:
>>
>> #pragma pack(push, 8) struct RTL_CRITICAL_SECTION { void *
>> reserved1; long reserved2; long reserved3;
>
> First, rather than using unsized types and assuming they are and
> will always remain the expected size, use typed sizes like int32_t.
> One could even collapse the structure further by using one int64_t
> in place of the two (assumed) 32 bit longs.  Given the use of void
> *'s for the other fields, presumably the structure's size varies
> between 32 and 64 bit platforms, so the same could not be done for
> those fields so neatly.

On one hand, we could look at it as "void*" and "long" are the 
underlying types that MS chose to use for <windows.h>, so they are 
correct by definition. On the other hand, even MS couldn't increase the 
size of a long when going to 64 bits due to exactly this kind of thing. 
So in practice, any compiler that wishes be able to compile Win32/64 
code is going to have to match the sizes of the basic types. Forever.

> Second, add a test to ensure that the size of the fabricated proxy
> is the same as the real thing.  That will ensure that the definition
> remains appropriate for all platforms present and future.

Seriously, windows.h considers itself the 900-pound gorilla. The 
inclusion of that header defines the platform, even to the point of 
requiring nonstandard language extensions. It was mostly solidified the 
early '90s. Most APIs are not particularly type-safe and there are many 
that are not even const-correct. I suggest not trying to reason about 
C++ best practices to the standard usually done with Boost projects.

> Since the test will be in compiled code that doesn't affect the
> header, it can safely include windows.h to get the real definition.

Doesn't worth an extra source project to me. If the size of such a 
simple and fundamental structure comes out wrong, so much other stuff 
isn't going to work that I doubt the compiler could even emit a loadable 
module.

On 10/11/2010 12:13 PM, Paul Blampspied wrote:
>
> Ray's point about MIN and MAX macros is well taken.  These macros
> caused me problems too.  I did not mention them because it is
> possible to avoid by defining NOMINMAX in the project file.

What's a "project file"? (Well, I know what you're talking about but 
it's not a standard feature of C++ and lots of stuff doesn't use them. 
E.g., Boost.)

Also, consider that there may be code somewhere that relies on them.

Include-once headers that change behavior based on preprocessor 
definitions are fundamentally evil.

> The Boost documentation does say that the use of Windows critical
> sections is an interim measure pending a portable Boost
> implementation of equivalent features.

They're going to need to use the underlying OS facilities one way or 
another. On Windows, that's kernel mutexes and critical sections (which 
can be faster).

> This means that it may not
> be worth making any changes to the library until the job can be done
> properly.  I suspect this won't be too long as Boost has recently
> increased it's portable support for thread synchronisation.

Rest assured that you are not the first, and will not be the last, to 
restate basic declarations to avoid brokenness introduced by the 
inclusion of windows.h.

Ideally, this stuff would accumulate under a permissive license (public 
domain even) so that others could re-use it.

- Marsh