3

I was compiling one of the projects I work with, this time with VS2010, just to find out that one of the includes in windows.h has a typedef INPUT which was clashing with an export const string in the code I already have.

//winuser.h (line: 5332)

typedef struct tagINPUT {
    DWORD   type;

    union
    {
        MOUSEINPUT      mi;
        KEYBDINPUT      ki;
        HARDWAREINPUT   hi;
    };
} INPUT, *PINPUT, FAR* LPINPUT;

//foo.h

//stuff here
extern FOO_DLL_API const string INPUT;

Now, I don't use INPUT in the offending .cpp (and I do not own most of the code), and trying to minimize the impact, I did the following:

//myfile.cpp

#include <foo.h>
namespace windowsLib {    //I added this
#  include <windows.h>
}

using namespace windowsLib;

So far, this approach is working fine, but I wanted to ask you if you see potential problems with this approach, or if you have a better suggestion.

Edit:

I appreciate all the comments and explanations on why this is a bad idea. What I get from your comments is that I should change foo.h and put the contents into a namespace. However, by doing that, I would be affecting dozens of files, and some more, which will now require namespace qualifications.

Is there a way to do this "the right way" without touching all those files?

If I was the owner of the code, I would do this change and be done with it, but I have to propose a solution and get it approved and assigned to someone, etc. So it will be easier if the change is minimal.

Edit 2:

My final proposal was to split the class in two as follows:

//stub.cpp

#include <windows.h>

//Implementation of wrapper methods

//stub.h

class stub {
    //public wrapper methods
}

//myfile.cpp

#include <stub.h>
#include <foo.h>

I'm accepting Benlitz answer since that suggestion would also solve the problem with the current minimal impact constrains I currently face. However, I thank you all for your comments.

Improve this question

4 Answers 4

Reset to default
3

Not only is it not allowed by the language (assuming standard headers will be included inside a namespace), there is also the problem of calling any functions declared in <windows.h> and finding that the linker will look for them in namespace windowsLib.

Just doesn't work!

Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

2

This seems like a bad idea at least.

1) If that's actually your code, there's no benefit from adding the namespace, since you have using namespace windowsLib; on the next line. Won't INPUT be ambiguous anyway?

2) You might include other headers that use stuff from windows.h and won't use the correct symbols. Imagine including a header that defines a function that returns INPUT. How's that gonna work out?

I suggest you play it safe and just rename your type.

Improve this answer

4 Comments

Yes INPUT remains ambiguous, but this class in particular is not using it. But granted, it may be used in the future. This "INPUT" constant is used all over the place (what was not used until recently is windows.h), so changing it would be a pain (not for me, because I don't maintain that code, but a pain neverless), so I would prefer to suggest them a solution with a limited impact, or that will prevent future problems.
Bare INPUT is now ambiguous (which is legal as long as it's disambiguated when used, either as ::INPUT or as windowsLib::INPUT), not a symbol with two different definitions (illegal). That said, bad idea. I recommend @Benlitz's approach.
@cvoinescu I don't recommend that approach. What's best is renaming the variable. That's just preprocessor abuse.
Of course, my bad. I misread his question -- I thought he said the other header was code he didn't have control over.
1

It is a bad idea as explained in other answers. Here is an idea that might help you fix your issue:

//myfile.cpp

#define INPUT UnusedSymbol
#include "foo.h"
#undef INPUT

#include "windows.h"

This might work because INPUT is an extern variable in foo.h, so neither the compiler nor the linker would care about it as long as you don't use it in myfile.cpp. UnusedSymbol is a dummy name, you can write whatever name is not used in your source.

Improve this answer

2 Comments

I tested this solution and it works. It may be, as others point out, that I shall push for a fix in foo.h instead, but this is a cleaver solution.
well yes, this answer is for the "minimizing the impact" request. I also advice you to put your foo.h into a namespace (by modifing foo.h itself of course, not writing the namespace declaration around an #include)
1

Putting a namespace around windows.h sounds dangerous to me. It probably would hide stuff you need. Except you import the namespace in the next line anyway.

I would sooner put the namespace around foo.h:

namespace fooLib {
#include "foo.h"
}

using fooLib;

I guess this just shifts the problem from OS code to foo code but that seems safer to me.

Another approach might be to build a wrapper around foo that calls foo functions and returns foo globals in a separate little wrapper library. One that didn't need windows.h. This wrapper you might put in a namespace to prevent this from happening again.

I'm assuming here that you don't have the ability to rename things in foo.h or put the fooLib namespace around stuff inside foo.h.

If you can touch foo.h it would be better to rename INPUT in foo.h or put foo.h stuff in a namespace of its own. I think a fooLib namespace would have great (obvious) benefit.

Improve this answer

1 Comment

I was digging into my old questions. Your comment about building a wrapper around foo is close to what I ended up proposing to the owners of the code. Since doing that with foo would be a pain, I encapsulated the windows.h dependency in a separate file and offered a wrapper class.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.