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)
Tracking
(Not tracked)
RESOLVED
WONTFIX
4.7.2
People
(Reporter: martijn.martijn, Assigned: wtc)
References
Details
Attachments
(2 files, 1 obsolete file)
794 bytes,
patch
|
Details | Diff | Splinter Review | |
2.64 KB,
patch
|
jag+mozilla
:
review+
|
Details | Diff | Splinter Review |
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>
Comment 1•17 years ago
|
||
I'm getting this as well, not really sure why.
Comment 2•16 years ago
|
||
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
Assignee | ||
Comment 3•16 years ago
|
||
Can one of you find out which MinGW header defines ntohl and change prinet.h to include it? Thanks.
Comment 4•16 years ago
|
||
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. */
Comment 5•16 years ago
|
||
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.
Assignee | ||
Comment 6•16 years ago
|
||
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.
Comment 7•16 years ago
|
||
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.
Reporter | ||
Comment 8•16 years ago
|
||
(In reply to comment #5) > Martijn: for a quick test, could you #include <winsock2.h> in > nsPNGDecoders.cpp? Yeah, thanks! That seems to help.
Reporter | ||
Comment 9•16 years ago
|
||
(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.
Comment 10•16 years ago
|
||
(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?
Comment 11•16 years ago
|
||
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.
Assignee | ||
Comment 12•16 years ago
|
||
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?
Comment 13•16 years ago
|
||
Yeah, MSVC has the same compilation error. I'm trying out a few things now.
Comment 14•16 years ago
|
||
Martijn, can you try the patch in bug 406580?
Assignee | ||
Comment 15•16 years ago
|
||
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
Comment 16•16 years ago
|
||
wtc: that's fine. I wouldn't mind a warning comment in prinet.h though.
Assignee | ||
Comment 17•16 years ago
|
||
Jag, you suggested adding a warning comment to prinet.h. How does this look?
Attachment #319121 -
Flags: review?(jag)
Assignee | ||
Comment 18•16 years ago
|
||
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 19•16 years ago
|
||
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+
Assignee | ||
Comment 20•16 years ago
|
||
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.
Description
•