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: Stewart, Robert (Robert.Stewart_at_[hidden])
Date: 2010-10-11 07:15:45

Marsh Ray wrote:
> On 10/09/2010 06:19 AM, Joshua Boyce wrote:
> >
> > #pragma pack(push, 8)
> >> typedef struct _RTL_CRITICAL_SECTION
> > {
> > void* DebugInfo;
> > long LockCount;
[snip]
> > } RTL_CRITICAL_SECTION, *PRTL_CRITICAL_SECTION;
> >> #pragma pack(pop)
>
> Suggestion:
>
> namespace boost { namespace boost_windows_h {
> #pragma pack(push, 8)
>       struct RTL_CRITICAL_SECTION
>       {
>               void * reserved1;
>               long reserved2;
>               long reserved3;
[snip]
>       };
>       typedef RTL_CRITICAL_SECTION * PRTL_CRITICAL_SECTION;
> #pragma pack(pop)
> } }
>
> The 'typedef struct' thing is a holdover from C.

> No reason to keep that struct tag, I don't think intended for
> use. Leading underscores are reserved for compiler vendors
> (like Microsoft) and organizations that ship non-standard
> headers (like Microsoft).
>
> The field data seems to change with OS versions, so code is not
> supposed to rely on it. The fields are effectively reserved.
>
> Personally, I'd use a lowercase struct name (it's not a macro
> after all) and omit the silly typedef. I'd be less concerned
> with it looking like the official headers if the point is to
> not include them. Just re-use the minimum needed for
> compatibility, which in this case is just sizeof() and
> __alignof().

Those suggestions appear reasonable, but there are a couple of
additional things that would be in order.

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.

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.  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.

_____
Rob Stewart                           robert.stewart_at_[hidden]
Software Engineer, Core Software      using std::disclaimer;
Susquehanna International Group, LLP  http://www.sig.com

IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.