config.h multiple inclusion causes compile errors on MinGW

[GoogleCodeExporter]

config.h is included by thread_cache.h and thread_cache.cc. Unfortunately, 
since mingw.h has a multiple inclusion header guard, anything it overrides 
gets reset on the second inclusion of config.h.

The solution is simple: add a header guard to config.h. Patch attached.

Original issue reported on code.google.com by neunon on 24 May 2010 at 3:10

Attachments:

• 0001-config.h-add-multiple-inclusion-guard.patch

[GoogleCodeExporter]

Nice catch.  Do you think maybe it's a better fix to just remove the header 
guards 
from mingw.h?  These files are supposed to be idempotent.  Do you see any 
problems 
with that solution?  I don't have particularly strong preferences one way or 
the 
other.

Original comment by csilv...@gmail.com on 24 May 2010 at 3:59

• Changed state: Accepted
• Added labels: Type-Defect, Priority-Medium

[GoogleCodeExporter]

I don't think that'd work, because mingw.h includes port.h, which _also_ 
potentially overrides a bunch of #defines 
from config.h. And that'd mean we'd need to remove header guards from port.h 
too. And that'd cause a lot of 
mayhem since port.h defines some structs and things.

Original comment by neunon on 24 May 2010 at 4:07

[GoogleCodeExporter]

port.h shouldn't be overriding #defines from config.h -- that should be the 
role of 
the windows/config.h, if necessary.  Do you have something specific in mind, 
where 
it's doing that?

Or maybe your point is just that port.h *could* do something like that, one 
day, so 
the no-header-guard solution is more fragile?

Original comment by csilv...@gmail.com on 24 May 2010 at 5:16

[GoogleCodeExporter]

In retrospect, I mean the latter. I saw a bunch of '#ifdef HAVE_*'s and 
_thought_ I saw a #define or #undef. But 
yes, you're right. Of course MSVC uses its own special config.h file in there. 
Maybe removing the mingw.h header 
guards isn't as terrible an idea as I thought.

Original comment by neunon on 24 May 2010 at 5:21

[GoogleCodeExporter]

Can you try it and see if it works for you?  (I don't know precisely what 
problems you 
were seeing that this fix solves.)  My bias right now is to take away the 
#ifdefs from 
mingw.h, and add a TODO to consider adding header guards for mingw.h + config.h 
if 
necessary.

Original comment by csilv...@gmail.com on 24 May 2010 at 5:49

[GoogleCodeExporter]

The problem I ran into was a redefinition of the 'struct timespec', because 
HAVE_PTHREAD was defined by 
./configure's config.h (which included something that redefined it). I did a 
#undef HAVE_PTHREAD in mingw.h 
only to find that HAVE_PTHREAD was still defined due to a later inclusion of 
config.h.

Removing the header guard on mingw.h trips a #error on port.h:47. If this 
#error is really something we need to 
consider, I think keeping the header guards in mingw.h is better than removing 
them, as we have no other way to 
indicate where port.h is being included. Removing that #ifdef/#error block 
allows it to build and pass the test 
suite, provided the patches I added to issue #95 are also applied to the code 
base.

Original comment by neunon on 24 May 2010 at 6:30

[GoogleCodeExporter]

We may not need the #error, but I think this is a sign that the 
no-header-guards 
solution is too fragile to use.  I'll add in header guards, like you suggest, 
for the 
next CL.

(btw, I think there must be something else going on with mingw and pthreads, 
which I 
don't understand yet, because I don't think the #undef you added should be 
necessary.  
But I agree this is an issue regardless of what happens with pthreads.)

Original comment by csilv...@gmail.com on 24 May 2010 at 7:18

• Changed state: Started

[GoogleCodeExporter]

This should be fixed in perftools 1.6, just released.

Original comment by csilv...@gmail.com on 5 Aug 2010 at 8:52

• Changed state: Fixed