Closed
Bug 308331
Opened 20 years ago
Closed 20 years ago
protypes.h guard for int32 on windows is way too agressive
Categories
(NSPR :: NSPR, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
4.6.5
People
(Reporter: timeless, Assigned: wtc)
References
Details
Attachments
(2 files)
935 bytes,
patch
|
wtc
:
review+
|
Details | Diff | Splinter Review |
700 bytes,
patch
|
timeless
:
review+
benjamin
:
superreview+
|
Details | Diff | Splinter Review |
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 dafydd@webchip.co.uk 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.
Attachment #195869 -
Flags: review?(wtchang)
*** Bug 307148 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 3•20 years ago
|
||
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•20 years ago
|
||
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.
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 ...)
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.
Assignee | ||
Comment 7•20 years ago
|
||
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.
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).
Assignee | ||
Updated•20 years ago
|
Attachment #195869 -
Flags: review?(wtchang) → review+
Assignee | ||
Comment 9•20 years ago
|
||
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).
Assignee | ||
Comment 10•20 years ago
|
||
I forgot to mention that the include-once guard that
MinGW's winsock2.h uses is _WINSOCK2_H, not
_WINSOCK2API_.
Assignee | ||
Updated•20 years ago
|
Attachment #196256 -
Attachment description: remove the guard → remove the guard (checked in)
Assignee | ||
Updated•20 years ago
|
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Target Milestone: --- → 4.6.3
Comment 11•19 years ago
|
||
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?
Assignee | ||
Updated•19 years ago
|
Attachment #196256 -
Flags: superreview?(benjamin)
Attachment #196256 -
Flags: review?(timeless)
Updated•19 years ago
|
Attachment #196256 -
Flags: superreview?(benjamin) → superreview+
Assignee | ||
Comment 12•19 years ago
|
||
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.
Assignee | ||
Comment 13•19 years ago
|
||
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
Target Milestone: 4.6.3 → 4.6.5
Reporter | ||
Comment 14•18 years ago
|
||
Comment on attachment 196256 [details] [diff] [review]
remove the guard (checked in)
sorry for the delay, fine by me.
Attachment #196256 -
Flags: review?(timeless) → review+
You need to log in
before you can comment on or make changes to this bug.
Description
•