vsnprintf macro breaks compilation on Windows

[GoogleCodeExporter]

What steps will reproduce the problem?

1. Try to complile perftools on Windows with MSVC. I use STLPort and MSVC 7.1, 
but I believe the problem should be present for other MSVC versions and, 
probably, for the stock STL.
2. Compilation fails within STL headers upon "using ::vsnprintf" directive into 
std namespace.

What is the expected output? What do you see instead?

Compilation should succeed.

What version of the product are you using? On what operating system?

Perftools 1.7 release. The problem is still present in SVN.

Original issue reported on code.google.com by andrey.s...@gmail.com on 19 Feb 2011 at 9:28

[GoogleCodeExporter]

The simple solution for the problem is to add the following to 
src/windows/port.h, right after the other includes:

  #ifdef __cplusplus
  #include <cstdio>
  #endif

The __cplusplus check is needed because this header is also included as pure C, 
and at that time it should not see cstdio.

However, a better solution would be to eliminate vsnprintf and other similar 
macros in this header. Probably, inline functions that forward to WinAPI and 
MSVC CRT would be a better solution.

Original comment by andrey.s...@gmail.com on 19 Feb 2011 at 9:34

[GoogleCodeExporter]

You're right, macros are evil, and it's no surprise that this one eventually 
bit us.

Would you be up for providing a patch that replaced all the #defines in port.h 
with small inline functions?  If not, I'll get to it when I have a chance.

Original comment by csilv...@gmail.com on 21 Feb 2011 at 12:23

• Added labels: Priority-Medium, Type-Defect

[GoogleCodeExporter]

I've attached a patch against trunk that replaces macros in port.h with 
functions. I wasn't able to replace vsnprintf because CRT already defines it, 
so I introduced perftools_vsnprintf instead and updated respective calls in the 
library to this new wrapper. Aside from that, pthread_once now behaves as it 
should and error handling is a bit more robust in a few places.

I tried this patched version with a few of our projects on MSVC 7.1, it showed 
no problems. I also compiled it with GCC 4.1.0 on Linux and MSVC 10.

PS: You may notice that I also updated the library version to 1.7. I can see a 
few places where the version is defined and it seems some of these places are 
easy to forget upon release. It would be nicer to have a single place for 
version definition.

Original comment by andrey.s...@gmail.com on 22 Feb 2011 at 8:02

Attachments:

• port.patch

[GoogleCodeExporter]

Thanks for the patch!  It looks great to me.  (I'd be a little happier if were 
possible to not have to change config.h.in, for non-windows users, but I'm not 
sure there's a great alternative.)  There a bit of style-formatting to be 
cleaned up, but otherwise it looks good to go in, to me.

But before I apply the patch, can you sign the CLA at
   http://code.google.com/legal/individual-cla-v1.0.html
?  Thanks!

Original comment by csilv...@gmail.com on 23 Feb 2011 at 2:33

[GoogleCodeExporter]

I had the idea of extracting the whole printf-thing into a separate header and 
include it everywhere regardless of the platform, but that looked like a too 
massive change and I wasn't sure you'd like to spread this portability glue to 
several headers.

> But before I apply the patch, can you sign the CLA

Done.

Original comment by andrey.s...@gmail.com on 23 Feb 2011 at 8:36

[GoogleCodeExporter]

Great, thanks.  I've applied the patch, made a few whitespace adjustments, and 
fixed up some of the changes so things also compile under mingw/msys.

The major change I had to make (for mingw) was to remove the throw/catch that 
you had in the pthread_once() code.  Hopefully it won't make much of a 
difference, since we've survived without that safety net so far...

I'm having my additional changes reviewed right now, and will then get the 
whole thing checked in.

Original comment by csilv...@gmail.com on 25 Feb 2011 at 1:11

• Changed state: Started

[GoogleCodeExporter]

This should be fixed in perftools 1.8, just released.

Original comment by csilv...@gmail.com on 16 Jul 2011 at 1:24

• Changed state: Fixed