308331 - protypes.h guard for int32 on windows is way too agressive

Status: RESOLVED FIXED

(NSPR :: NSPR, defect)

Description

[timeless]

for my reference, there are two google hits indicating that winsock2.h really 
sometimes does have this problem, one is: 
http://web.archive.org/web/20030512232603/http://techweb.rfa.org/pipermail/porta
udio/2002-December/001372.html

(that should last forever),

the other:
http://techweb.rfa.org/pipermail/portaudio/2002-December/001372.html

Dafydd James [e-mail] Tue, 3 Dec 2002 15:19:55 -0000 

> I'm using Dev-C++ with MinGW, and when I try to compile my project with
> portaudio files included, I get the following:
> 71 X:\Documents\dev\C++\DSP01\portaudio\pa_win_wmme.c
> In file included from portaudio/pa_win_wmme.c
> 52 X:\Documents\dev\C++\DSP01\portaudio\pa_host.h
> redefinition of `int32'
> 662 C:\Software\Dev-Cpp\include\winsock2.h
> `int32' previously declared here
> 285 X:\Documents\dev\C++\DSP01\portaudio\pa_win_wmme.c

--
i've checked, msvc1.5 .. msvc4.1 do not have winsock2.h

none of: msvc6, 7.0, 7.1, current psdks, 
ftp://ftp.microsoft.com/bussys/winsock/winsock2/winsock2.h have this problem.

note that most mozilla devs don't run into this problem because they don't 
target nt5, so by default windows.h pulls in winsock instead of winsock2.

thanks to luser, it seems that msvc4.2 does have that typedef its winsock2.h is 
version 2.1.x. i'm still looking for someone w/ msvc5.

Comment 1

[timeless]

Attachment: fix guard to match actual problem case

Comment 2

[timeless]

*** Bug 307148 has been marked as a duplicate of this bug. ***

Comment 3

[Wan-Teh Chang]

Comment on attachment 195869 [details] [diff] [review]
fix guard to match actual problem case

>+/* winsock 2.1.x - msvc4.2 defines its own "int32"

Could you email me a copy of the winsock2.h file that
defines its own "int32"?

>+ * winsock 2.2.x define WINSOCK_VERSION and don't typedef int32

Thank you very much for doing the research.

If we don't need to support MSVC 4.2, we can just remove
the ifdef below:

>+#if !defined(WIN32) || !defined(_WINSOCK2API_) || \
>+    defined(WINSOCK_VERSION)

Comment 4

[John Gaunt (redfive)]

I'm not sure all this is accurate of my situation. i am seeing this on both
WinXP and WinNT using MSVC6 with the latest patches and processor packs. I am
targeting 0x400 (not 0x500 as was stated in a bug that was duped to this one),
according to my build system( WINVER 0x400 and _WIN32_WINNT 0x400 on both systems).

Both systems have winsock2.h and are loading that which is causing the ultimate
failure in the building of webshell/tests/viewer, as timeless has hunted down. I
have found no typdef-ing of int32 in any of my msvc headers.

I am very curious about why this just popped up. I was building just fine for
awhile and without changing either system I have begun to see this error within
the last month or so. I wish I had paid more attention to when it started, but I
figured it was just a checkin bug or an annomoly on my systems.

Comment 5

[timeless]

sorry, the other missing piece is that redfive has a newer platformsdk, and 
that newer platform sdk's windows.h conditionally pulls in winsock2.h or 
winsock.h older platform sdk's (e.g. the one that ships w/ msvc7.1) always pull 
in winsock.h (which of course does not define a WINSOCK2 guard ...)

Comment 6

[timeless]

ok, minor correction, msvc6's windows.h is the one that conditionally includes 
winsock2/winsock not newer ones... *sigh*.

anyway, i'd rather leave support for vc42, it's harmless.

Comment 7

[Wan-Teh Chang]

timeless: if you have MSVC 4.2 installed, could you
test this code?

typedef long int32;
typedef long int32;

Compile it with "cl /W4 /c foo.c".  See if it
compiles without any error or warning.

MSVC 6 and 7.1 allow duplicate typedefs.  If MSVC 4.2
also allows that, we can take advantage of that and
do a simpler fix -- simply remove

  #if !defined(WIN32) || !defined(_WINSOCK2API_)

and the matching

  #endif

around the int32 typedefs in protypes.h.

Comment 8

[timeless]

i don't have vc42, i had to find someone who did. i'm not sure he has it
installed. i do have access to vc15, but i managed to destroy a critical
directory wednesday morning so i'm going to be spending time rebuilding my
mozilla world and doing work instead of doing stuff like this.

actually. simply put, your argument is faulty. it's not just vc42 that matters,
it's any compiler that works w/ nspr that is fed winsock2.1. this would include
gcc (version 3.3 most definitely complains).

Comment 9

[Wan-Teh Chang]

Attachment: remove the guard (checked in)

I installed MSVC 4.0 and 4.2, and w32api-2.2.tar.gz for MinGW (the
oldest version of w32api I can find).

I concluded that the guard for the int32 typedef is no longer
necessary.  MSVC 4.0 doesn't have winsock2.h.  I confirmed timeless's
research that only the winsock2.h file (winsock version 2.1) in MSVC
4.2 has the int32 typedef.  MinGW's winsock2.h doesn't have that typedef
(it replaces int32 with unsigned int), and current Microsoft winsock2.h
doesn't have that typedef either.  Also, on Windows, protypes.h defines
int32 as long (as opposed to PRInt32), which is the same as the
int32 typedef in winsock2.h v2.1.  MSVC accepts such duplicated typedefs.

Also, the guard we have only works if winsock2.h is included before
protypes.h is included.  It doesn't protect the other way around.
So the guard is not bullet proof.

I have thought about every case and concluded that it is safe to
simply remove the guard.

Since I am the last active NSPR developer who still remembers the
history of this guard, I decided to invoke my NSPR module owner's
privilege and remove the guard on the NSPR trunk (NSPR 4.7)
and NSPRPUB_PRE_4_2_CLIENT_BRANCH (mozilla1.9a).

Comment 10

[Wan-Teh Chang]

I forgot to mention that the include-once guard that
MinGW's winsock2.h uses is _WINSOCK2_H, not
_WINSOCK2API_.

Comment 11

[Benjamin Smedberg]

This is still biting the Mozilla 1.8 branch, which is using NSPR_4_6_4_RTM. (The error appears when tests are enabled in webshell/tests/viewer/JSConsole.cpp. Is there a way to get this backported?

Comment 12

[Wan-Teh Chang]

Ben, if you compile webshell/tests/viewer/JSConsole.cpp with
-DNO_NSPR_10_SUPPORT, does that work?  That'd be one way to
work around this bug.

Comment 13

[Wan-Teh Chang]

I checked in the patch "remove the guard (checked in)" on the
NSPR_4_6_BRANCH (NSPR 4.6.5).

Checking in protypes.h;
/cvsroot/mozilla/nsprpub/pr/include/obsolete/protypes.h,v  <--  protypes.h
new revision: 3.18.2.1; previous revision: 3.18
done

Comment 14

[timeless]

Comment on attachment 196256 [details] [diff] [review]
remove the guard (checked in)

sorry for the delay, fine by me.