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