Hiding the <windows.h>

[Badestrand]

Hi!

I just have a suggestion how to get rid of the windows.h-includes in the sfml-header-files (like in thread.hpp or mutex.hpp).

The problem I have is that the windows.h massively pollutes the global namespaces, first by hundreds of functions and types and second, even worse, by dozens of macros like "max" (which prohibits the use of std::max).


The solution I propose is the following:

The windows.h in the header-files is needed only (as far as I see it) for win-types like "HANDLE" or "CRITICAL_SECTION" which are class-members of classes like sf::Thread or sf::Mutex.
So, if we want to get rid of the windows.h in the header-files, we'll have to replace those variable-types by types which aren't declared in the windows.h.
Replacing the HANDLE-type is easy since it's just a typedef to a pointer to something. We just would have to replace "HANDLE" in the header-file by "void*", include the windows.h in the cpp-file instead of the header and cast around (cast a lot, I admit) the void* to HANDLE in the cpp-file at every access to this variable.

Replacing ODTs like CRITICAL_SECTION is quite more complicated since this is a structure with actual members. My suggestion is to declare a private binary-equal structure inside the class; the substitude-structure of course must contain types like long, int and whatever. Since the winapi-functions always take pointers to structures like CRITICAL_SECTION, a simple cast again should do the thing as long as our replacement-structure is at least as big (in size in bytes) as the original CRITICAL_SECTION.

So: What do you think of this?


edit: I suppose that replacing the "CRITICAL_SECTION"-type could be done by just using "char[24]" and something equal to "BOOST_STATIC_ASSERT( sizeof(cs_mem_var) >= sizeof(CRITICAL_SECTION )" in the implementation-file.

edit: As far as I can see, only 3 header-files include the windows.h (SocketHelper, Mutex, Thread). I could do it and send someone the new sources if someone would like that.

edit: Ok, at least the the min- and max- macros from windows.h are prohibited with #define NOMINMAX, nevertheless there are enough other macros.

[Badestrand]

This got a little more complicated than I thought, many files need to be changed. For example, SocketTCP needs something like

Code: [Select]


#if defined(SFML_SYSTEM_WINDOWS)
#include <winsock2.h>
#endif


Anyhow, I think it's worth.

[Laurent]

Funny, someone asked this on the french forum 2 days ago.

Getting rid of windows.h in public headers would be straight-forward, the only reason I keep windows.h publicly included is that gl.h needs it. And including OpenGL headers publicly is done on purpose in SFML, I won't get rid of them.

I know windows.h really pollutes the global namespace, but there's no way to avoid it when using OpenGL. The best solution is to create a header which #undef all the Win32 symbols you need to use.

[Badestrand]

Would it be possible to ban the windows.h at least from the packages where OpenGl is not used? I personally use the system and network packages only so there should be no need for OpenGL.

[Laurent]

Well as you said, there's SocketHelper.hpp which also needs winsock2.h. It's needed for the SOCKET type definition, which relies on non-standard 64 bits compiler extensions so it can't be easily replaced.

[Badestrand]

It's needed for the SOCKET type definition, which relies on non-standard 64 bits compiler extensions so it can't be easily replaced.

I don't think so. Following the SOCKET-definition you can find

Code: [Select]

typedef UINT_PTR  SOCKET;

If we believe a comment in my basetsd.h-file

Code: [Select]

// The INT_PTR is guaranteed to be the same size as a pointer.  Its
// size with change with pointer size (32/64).  It should be used
// anywhere that a pointer is cast to an integer type. UINT_PTR is
// the unsigned variation.
, then SOCKET is as wide as a pointer so it should be possible to represent the SOCKET-type be void*.


Btw, following the definition for UINT_PTR you can find

Code: [Select]

#if defined(_WIN64)
    typedef unsigned __int64 UINT_PTR;
#else
    typedef _W64 unsigned int UINT_PTR, *PUINT_PTR;
#endif

// with

#if !defined(__midl) && (defined(_X86_) || defined(_M_IX86)) && _MSC_VER >= 1300
    #define _W64 __w64
#else
    #define _W64
#endif

// _MSC_VER>=1300 is Visual C++ .NET 2002 upwards (see http://forums.msdn.microsoft.com/en/vcgeneral/thread/17a84d56-4713-48e4-a36d-763f4dba0c1c/)

__w64 is just a compiler-extension which doesn't influence the type it's used with:

The __w64 keyword lets you mark variables, such that when you compile with /Wp64 the compiler will report any warnings that would be reported if you were compiling with a 64-bit compiler.

Any typedef that has __w64 on it must be 32 bits on x86 and 64 bits on ia64.

The __w64 modifier should be specified on any typedefs that change size between 32 bit and 64 bit platforms. For any such type, __w64 should appear only on the 32-bit definition of the typedef.

[Laurent]

I usually don't like substituting system specific types with primitive ones, but it seems you've strongly checked this one and apparently it's safe to replace with a pointer type, so I'll give it a try.

Thanks for your help

[Badestrand]

I usually don't like substituting system specific types with primitive ones, but it seems you've strongly checked this one and apparently it's safe to replace with a pointer type, so I'll give it a try.

Juchu, great!

Thanks for your help

It's also my profit


But there's another one: fd_set in SelectorBase.hpp in which the SOCKET-type is also used. Generally it should be no problem to substitude fd_set's SOCKET with the pointer but I don't know how or whether it conflicts with the linux or mac version since this substitution needs to take place in the SelectorBase.hpp-file. Maybe you'll need an plattform-dependent extra-include for such types? I guess this also would it make simpler to switch between the windows.h-version and the substitution-version, something like:

Code: [Select]


// SystemSpecifics.hpp for the windows-compilement
#define MAKE_SUBSTITUTIONS

#ifdef MAKE_SUBSTITUTIONS
    typedef void* HANDLE;
    typedef void* SOCKET;
    typedef ... fd_set;
#else
    #include <winsock2.h>
    #include <windows.h>
#endif

But I didn't think long enough about this so there may be some contras...

[Laurent]

I'll take a look at this stuff as soon as I have some free time.

[Badestrand]

- empty post -