Closed Bug 411055 Opened 17 years ago Closed 16 years ago

Mingw build error, error: `ntohl' was not declared in this scope

Categories

(NSPR :: NSPR, defect)

x86
Windows XP
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: martijn.martijn, Assigned: wtc)

References

Details

Attachments

(2 files, 1 obsolete file)

After solving the build issue of bug 410439, by applying Jag's patch from bug 406580, I'm getting this Mingw build error instead:

DSTDC_HEADERS=1 -DHAVE_DIRENT_H=1 -DHAVE_GETOPT_H=1 -DHAVE_MEMORY_H=1 -DHAVE_UNI
STD_H=1 -DHAVE_MALLOC_H=1 -DHAVE_LIBM=1 -DNO_X11=1 -DMMAP_MISSES_WRITES=1 -DHAVE
_STRERROR=1 -DHAVE_SNPRINTF=1 -DHAVE_MEMMOVE=1 -DHAVE_RINT=1 -DVA_COPY=va_copy -
DHAVE_VA_COPY=1 -DMOZ_EMBEDDING_LEVEL_DEFAULT=1 -DMOZ_EMBEDDING_LEVEL_BASIC=1 -D
MOZ_EMBEDDING_LEVEL_MINIMAL=1 -DMOZ_PHOENIX=1 -DMOZ_BUILD_APP=browser -DMOZ_XUL_
APP=1 -DMOZ_DEFAULT_TOOLKIT=\"cairo-windows\" -DMOZ_THEBES=1 -DMOZ_CAIRO_GFX=1 -
DMOZ_DISTRIBUTION_ID=\"org.mozilla\" -DOJI=1 -DIBMBIDI=1 -DMOZ_VIEW_SOURCE=1 -DM
OZ_XPINSTALL=1 -DMOZ_JSLOADER=1 -DNS_PRINTING=1 -DNS_PRINT_PREVIEW=1 -DMOZ_NO_XP
COM_OBSOLETE=1 -DMOZ_XTF=1 -DMOZ_MATHML=1 -DMOZ_ENABLE_CANVAS=1 -DMOZ_SVG=1 -DMO
Z_SVG_FOREIGNOBJECT=1 -DMOZ_UPDATE_CHANNEL=default -DMOZ_PLACES=1 -DMOZ_FEEDS=1
-DMOZ_STORAGE=1 -DMOZ_SAFE_BROWSING=1 -DMOZ_URL_CLASSIFIER=1 -DMOZ_LOGGING=1 -DH
AVE___CXA_DEMANGLE=1 -DMOZ_DEMANGLE_SYMBOLS=1 -DHAVE__UNWIND_BACKTRACE=1 -DMOZ_U
SER_DIR=\"Mozilla\" -DHAVE_STDINT_H=1 -DHAVE_INTTYPES_H=1 -DHAVE_UINT64_T=1 -DMO
Z_XUL=1 -DMOZ_PROFILELOCKING=1 -DMOZ_RDF=1 -DMOZ_MORKREADER=1 -DMOZ_DLL_SUFFIX=\
".dll\" -DJS_THREADSAFE=1 -DMOZ_REFLOW_PERF=1 -DMOZ_REFLOW_PERF_DSP=1 -DMOZILLA_
LOCALE_VERSION=\"1.9a1\" -DMOZILLA_REGION_VERSION=\"1.9a1\" -DMOZILLA_SKIN_VERSI
ON=\"1.8\"  -D_MOZILLA_CONFIG_H_ -DMOZILLA_CLIENT /cygdrive/c/mozilla/mozilla/mo
dules/libpr0n/decoders/png/nsPNGDecoder.cpp
c:/mozilla/mozilla/modules/libpr0n/decoders/png/nsPNGDecoder.cpp: In function `v
oid info_callback(png_struct*, png_info*)':
c:/mozilla/mozilla/modules/libpr0n/decoders/png/nsPNGDecoder.cpp:595: warning: c
omparison between signed and unsigned integer expressions
c:/mozilla/mozilla/modules/libpr0n/decoders/png/nsPNGDecoder.cpp:595: warning: c
omparison between signed and unsigned integer expressions
c:/mozilla/mozilla/modules/libpr0n/decoders/png/nsPNGDecoder.cpp: In function `v
oid row_callback(png_struct*, png_byte*, png_uint_32, int)':
c:/mozilla/mozilla/modules/libpr0n/decoders/png/nsPNGDecoder.cpp:741: error: `nt
ohl' was not declared in this scope
make[6]: *** [nsPNGDecoder.o] Error 1
make[6]: Leaving directory `/cygdrive/c/mozilla/mozilla/_firefox/modules/libpr0n
/decoders/png'
make[5]: *** [libs] Error 2
make[5]: Leaving directory `/cygdrive/c/mozilla/mozilla/_firefox/modules/libpr0n
/decoders'
make[4]: *** [libs] Error 2
make[4]: Leaving directory `/cygdrive/c/mozilla/mozilla/_firefox/modules/libpr0n
'
make[3]: *** [libs_tier_gecko] Error 2
make[3]: Leaving directory `/cygdrive/c/mozilla/mozilla/_firefox'
make[2]: *** [tier_gecko] Error 2
make[2]: Leaving directory `/cygdrive/c/mozilla/mozilla/_firefox'
make[1]: *** [alldep] Error 2
make[1]: Leaving directory `/cygdrive/c/mozilla/mozilla/_firefox'
make: *** [alldep] Error 2

C:\mozilla\mozilla>
I'm getting this as well, not really sure why.
It's because prinet.h doesn't provide ntohl in a mingw environment. Moving this to the NSPR product/component.
Assignee: nobody → wtc
Component: GFX: Thebes → NSPR
Product: Core → NSPR
QA Contact: thebes → nspr
Version: Trunk → 4.7.3
Can one of you find out which MinGW header defines ntohl and
change prinet.h to include it?  Thanks.
Is there a special MinGW header, or can we just include winsock.h or winsock2.h?

wtc, why don't we #include <winsock.h> (or winsock2.h) for WIN32?

104 #elif defined(WIN32)
105 
106 /* Do not include any system header files. */
Martijn: for a quick test, could you #include <winsock2.h> in nsPNGDecoders.cpp?

You also might need to add ws2_32 to the Makefile.in:

ifeq (WINNT,$(OS_ARCH))
HOST_EXTRA_LIBS += $(call EXPAND_LIBNAME,ws2_32)
endif

Though hopefully not, since the function should just be inlined.
I see.  If prinet.h doesn't include any system headers
for regular Win32, it should not include a system header
for MinGW.  prinet.h is not documented to provide the
ntohl macro, so it doesn't need to do that.  You need
to get the ntohl macro definition some other way.  NSPR
does provide PR_ntohl, which is a function as opposed to
a macro.

It seems that you should get the same compilation error
with nsPNGDecoder.cpp for regular WIN32.

The reason no NSPR header files include <windows.h> or
<winsock.h> (or <winsock2.h>) is to speed up compilation.
I can't really include nspr.h and use |PR_ntohl()| in gfxColor.h because it won't get inlined.

I understand that prinet.h doesn't provide a |ntohl()| macro, but according to the header blurb:

 42  *     This file serves the following purposes:
 43  *     - A cross-platform, "get-everything" socket header file.
...
 47  *     - NSPR needs the following macro definitions and function
 48  *       prototype declarations from these header files:
 51  *           ntohl(), ntohs(), htonl(), ntons().
 52  *       NSPR does not define its own versions of these macros and
 53  *       functions.  It simply uses the native versions, which have
 54  *       the same names on all supported platforms.

The way I read that, prinet.h's purpose is to make sure that whoever includes it (indirectly through prio.h) has access to |ntohl()| (whether a macro or function). It doesn't look like it's actually doing that for Windows (VC++ nor MinGW).

I guess including winsock.h in here means it's (indirectly) included by pretty much all files, and that would slow down compilation quite a bit. I don't really see how to make it so that prinet.h provides access to |ntohl()| on all platforms without slowing down compilation a ton. I guess that makes this bug a WONTFIX :-(

So how do all these places that use |ntohl()| and friends actually compile?

prnetdb.c (PR_ntohl() definition which uses ntohl()) includes primpl.h, which includes prosdep.h, which includes _winnt.h, which includes winsock.h

The places outside NSPR that use |ntohl()| directly #include winsock.h or winsock2.h. We could probably do the same in gfxColor.h.

wtc, leaving this open for now. I would really like to be able to #include prinet.h (or rather, prio.h) and have access to |ntohl()| etc. on all platforms. I'm hoping maybe you see a way to address this, but feel free to WONTFIX it.
(In reply to comment #5)
> Martijn: for a quick test, could you #include <winsock2.h> in
> nsPNGDecoders.cpp?

Yeah, thanks! That seems to help.
Attached patch patchSplinter Review
(In reply to comment #6)
> The reason no NSPR header files include <windows.h> or
> <winsock.h> (or <winsock2.h>) is to speed up compilation.

That's not a big issue, since building with mingw is already as slow as it can be.
(In reply to comment #9)
> > The reason no NSPR header files include <windows.h> or
> > <winsock.h> (or <winsock2.h>) is to speed up compilation.
> 
> That's not a big issue, since building with mingw is already as slow as it can
> be.

Speed may not be important for mingw, but your patch as-is would affect all compilers :) Perhaps you should add a #if MINGW (or whatever the right constant is) conditional?
Martijn: right, so that's not the right fix of course. You'd want to add this to gfxColor.h, inside a #ifdef XP_WIN section. Except for some reason that confuses the Windows build. Pulling in winsock.h on the TryServer causes a compilation error in nsSVGFilter.cpp because it can't find LoadImage (my guess is that gfxColor.h gets included somewhere before nsImageLoadingContent.h gets included somewhere, and through winsock.h we somehow pull in a #define LoadImage LoadImageA or something).

Anyway, doing a TryServer build now with winsock2.h, and if that fails, I'll try throwing in a #undef LoadImage right there in gfxColor.h

Though really, I'm now thinking about moving all of this out to a new header file so just the few places that want the GFX_0XFF_PPIXEL_FROM_BPTR macro can #include that.
Jag: prinet.h should have been an NSPR internal header and
included winsock.h.  But it is a public header by mistake.
It used to include windows.h but we removed it at customer's
request (in rev. 3.1.10.1/rev. 3.2) to speed up compilation.
So now it's left in this inconsistent state.

I suggest that you work around this issue by including these
headers in your code:

#include "prio.h"
#ifdef WIN32
#include <windows.h>
#endif

This emulates rev. 3.1 of prinet.h.

I believe your code should have the same compilation error
with MSVC on Windows, right?
Yeah, MSVC has the same compilation error.

I'm trying out a few things now.
Martijn, can you try the patch in bug 406580?
jag: since you found a solution for bug 406580, I'm marking
this bug WONTFIX.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → WONTFIX
Target Milestone: --- → 4.7
Version: 4.7.3 → other
wtc: that's fine. I wouldn't mind a warning comment in prinet.h though.
Jag, you suggested adding a warning comment to prinet.h.
How does this look?
Attachment #319121 - Flags: review?(jag)
Minor edits.  I'm still not sure if I get the message across...
Attachment #319121 - Attachment is obsolete: true
Attachment #319122 - Flags: review?(jag)
Attachment #319121 - Flags: review?(jag)
Comment on attachment 319122 [details] [diff] [review]
Add a warning comment to prinet.h, v2

wtc: that looks good to me. r=jag
Attachment #319122 - Flags: review?(jag) → review+
I checked in the prinet.h comment patch on the NSPR trunk (NSPR 4.7.2).

Checking in prinet.h;
/cvsroot/mozilla/nsprpub/pr/include/prinet.h,v  <--  prinet.h
new revision: 3.14; previous revision: 3.13
done
Target Milestone: 4.7 → 4.7.2
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: