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-11 09:35:34

On Mon, Oct 11, 2010 at 10:15 PM, Stewart, Robert <Robert.Stewart_at_[hidden]>wrote:

> 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.
> _______________________________________________
> Unsubscribe & other changes:
> http://lists.boost.org/mailman/listinfo.cgi/boost
>

Microsoft document the sizes of various types on MSDN, and given the
backwards-compatibility guarantees offered by the Windows API they can't
really change them.

http://msdn.microsoft.com/en-us/library/s3f49ktz.aspx
http://msdn.microsoft.com/en-us/library/cc953fe1.aspx

Not to say that it would be a bad idea to use sized types 'just in case'.
I'm all for it. I'm just saying, all the Windows compilers I use (MSVC, GCC
via MinGW, and ICC) 'adhere' to those size lists, and I can't really see a
situation where that would change, as it would break so much existing code.

Sorry if I'm not being very articulate. Quite tired, and about to hit the
sack.

P.S. There's no reason to use sized types for the arguments which we're
using pointers for, as the 'real' types are just DWORD_PTR/ULONG_PTR/etc
which are guaranteed to be the same size as a pointer anyway.