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: Joshua Boyce (raptorfactor_at_[hidden])
Date: 2010-10-09 07:19:52

On Sat, Oct 9, 2010 at 5:04 AM, Marsh Ray <marsh_at_[hidden]> wrote:

> On 10/08/2010 12:03 PM, Paul Blampspied wrote:
>
>>
>>>> typedef char CRITICAL_SECTION[24]; //24 is sizeof(CRITICAL_SECTION) !
>>>>
>>>
>>> Can you guarantee that this is the correct size
>>> on all systems?  Are you sure that there are
>>> no alignment problems?
>>>
>>
>> Yes, it does link, and appears to work, at least under Vista.  I took the
>> declaration directly from Windows API documentation.
>>
>
> My guess is that reflects 3 or 6 uintptr_t values (I didn't see if it was
> x86 or x64).
>
> However, nearly every Windows program in the world has this size and
> alignment dependency compiled into it. Once you get the correct requirements
> (or larger), they won't change.
>
>
>  I could not find a
>> declaration for CRITICAL_SECTION.
>>
>
> It's probably in the DDK in detail, but must be in the normal SDK
> somewhere.
>
>
>  I am sure there are better ways of doing this.
>>
>
> Not really :-(
>
> Typically, the developer supplying a header files for a Windows library
> assumes the app developer will have already included windows.h.
>
>
>  I think the important thing
>> is to avoid including windows.h, which defines a huge number of symbols
>> and
>> can break code.
>>
>
> Yes, absolutely. It also doesn't compile as standard C or C++.
>
> As a long-time Win32 programmer who's run into this many times my
> conclusion is give up on the idea of including windows.h anywhere except
> consistently at the top of every translation unit that possibly needs it.
> Windows apps universally use precompiled headers for this and any other
> order of inclusion isn't well supported.
>
> This is also a general problem with any header that specifies "#define X
> before #including this header so it will behave a certain way". I'm
> surprised it's not a well known coding no-no.
>
>
>  One particular nasty is the __in_range macro which
>> conflicts with stlport's  checked iterators in debug builds.  A less risky
>> change might be to simply undefine some of the undocumented macros that
>> windows.h leaves behind it.  A judicious use of namespaces allows you to
>> avoid most of the other problems.
>>
>
> And MIN and MAX.
>
> I'll never forget the weird compile error I got when defined my own thing
> called TOKEN_TYPE.
>
> - Marsh
>
> _______________________________________________
> Unsubscribe & other changes:
> http://lists.boost.org/mailman/listinfo.cgi/boost
>

Your code won't work when compiled for x64.

Here's the proper definition (Ripped from WinNT.h):

 #pragma pack(push, 8)
>
>
>> typedef struct _RTL_CRITICAL_SECTION {
>
>     PRTL_CRITICAL_SECTION_DEBUG DebugInfo;
>
>
>>     //
>
>     //  The following three fields control entering and exiting the
>> critical
>
>     //  section for the resource
>
>     //
>
>
>>     LONG LockCount;
>
>     LONG RecursionCount;
>
>     HANDLE OwningThread;        // from the thread's ClientId->UniqueThread
>
>     HANDLE LockSemaphore;
>
>     ULONG_PTR SpinCount;        // force size on 64-bit systems when packed
>
> } RTL_CRITICAL_SECTION, *PRTL_CRITICAL_SECTION;
>
>
>> #pragma pack(pop)
>
>
HANDLE is always the size of a pointer. LONG is always 4-bytes under
Windows. ULONG_PTR is the size of a pointer. PRTL_CRITICAL_SECTION_DEBUG  is
obviously the size of a pointer. Also, notice the explicit packing.

So, a definition which does not required the Windows headers would be as
follows:

#pragma pack(push, 8)

> typedef struct _RTL_CRITICAL_SECTION

{

void* DebugInfo;

long LockCount;

long RecursionCount;

void* OwningThread;

void* LockSemaphore;

void* SpinCount;

} RTL_CRITICAL_SECTION, *PRTL_CRITICAL_SECTION;

> #pragma pack(pop)