On Linux, including <string.h> tends to cause lots of warnings because it defines index (another name for strchr), and lots of people like to use that name for their variables. The warning is: warning: declaration of `index' shadows global declaration We're including string.h in the following header files exported to dist/include. In most cases there's no need to include it in the header file at all; it should be included in the .cpp file instead. extcache.h interpreter.h jsarena.h nsCRT.h nsID.h nsTarget.h seccomon.h I suspect the one causing the most problems (included in the most places) is nsID.h, with nsCRT.h second. nsCRT.h in its current form does need the inclusion because it has inlines to call various system functions. In order to cut down on warnings, we would need to decide whether to stop including string.h unless necessary, or not name variables "index".
scc is new owner of string stuff, but I think the bug here is the gratuitous nested include. Only if the header itself required a type size or name from string.h should it include that file. I'm reassigning to dp and cc'ing scc and warren, the XPCOM troika who can fix most of the offending .h files you found. Either other bugs should be filed, or this bug should be passed around, till all gratuitous <string.h> includes have been removed. I'm removing the gratuitous include of <string.h> from jsarena.h, which shouldn't have been there, and wasn't back in the day (when it was prarena.h). Fur stop me if I'm doing wrong. /be
Don't forget to check it in on SM140_BRANCH too.
Ugh! what is SM140_BRANCH
I've got a build here without <string.h> in nsID.h, and all went well. I'm going to check it in shortly and see how many of these warnings are left.
Shaver you want to take this bug over ?
This is cleanup. Moving to post beta.
Two points: (1) |index| is likely to live wherever |memchr| lives (meaning <string.h>, or perhaps <cstring> on most systems), and because |memchr| is used in inline functions in .h files, e.g., "nsCharTraits.h", there will be trouble; (2) |index| is not, AFAIK, part of the C or C++ standard; it is a platform extension synonym for |memchr| in a few places. Perhaps the right way to fix this is to fix the header file? Get better headers? Someday, of course, we'll be using namespaces and problems like this will less frequent. How bad is it to fix the header file and remove the non-standard global namespace polluting declaration?
How bad is it to tell developers that they all need to go edit their system include files to get rid of the reference to index (which some other program might actually use?) I don't think most people will willing to do that.
If we could minimize the number of places that include <string.h>, then we could do this in all those places: #define index _pre_ansi_invasive_index #include <string.h> #undef index and (no one linking with Mozilla code should ever call index, right?) we'd be free of those annoying warnings. Or am I missing something? /be
Yes, that would be a fine solution, and in fact minimizing the number of places that include <string.h> was my intent in filing this bug in the first place, since few files should need to include it (they should usually be using nsCRT.h or nsString.h or some other utility).
Sorry if obvious: don't make jsarena.h include any xpcom header file, or any header outside of js/src -- the JS engine must stand alone. So ifdef ugliness will have to infest at least two (three if interpreter.h is Java) places. /be
mass re-assigning to my new bugzilla account
dp is no longer @netscape.com changing qa contact to default for this component
[bugzilla component to be deleted]
apparently, no one has actually cared about this bug for 2 years