Should make global data const where possible

NEW
Unassigned

Status

()

defect
--
trivial
19 years ago
5 years ago

People

(Reporter: sfraser_bugs, Unassigned)

Tracking

({helpwanted})

Trunk
mozilla1.9.2a1
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [ToDo: bitrotted patches, comment 85])

Attachments

(15 attachments, 5 obsolete attachments)

8.28 KB, patch
waterson
: superreview+
Details | Diff | Splinter Review
1.88 KB, patch
sfraser_bugs
: superreview+
Details | Diff | Splinter Review
4.69 KB, patch
dbaron
: review+
Details | Diff | Splinter Review
87.45 KB, patch
smontagu
: review+
Details | Diff | Splinter Review
1.38 KB, patch
Details | Diff | Splinter Review
1.11 KB, patch
Details | Diff | Splinter Review
42.65 KB, patch
Details | Diff | Splinter Review
16.89 KB, patch
Details | Diff | Splinter Review
1.76 KB, patch
dougt
: review+
Details | Diff | Splinter Review
10.66 KB, patch
Details | Diff | Splinter Review
15.81 KB, patch
darin.moz
: review+
Details | Diff | Splinter Review
32.47 KB, patch
dbaron
: review+
Details | Diff | Splinter Review
1.63 KB, patch
brendan
: review+
beltzner
: approval1.9.1-
Details | Diff | Splinter Review
21.54 KB, patch
Details | Diff | Splinter Review
5.91 KB, patch
dbaron
: review+
Details | Diff | Splinter Review
Making global data const has payoffs in terms of compiler optimizations, and the 
ability of the linker to put global data in read-only sections of the DLL, that 
has consequences for file mapping.
Other places that could be fixed (const char*'s):
<http://lxr.mozilla.org/seamonkey/search?string=const+char*>
Search in the results for '[]' for the bigger wins. There are some in xpconnect 
and JS, and a few in libpref.

For the benefits of making global data const, read this *excellent*
PDF on Mac OS X performance evaluation.

<http://developer.apple.com/techpubs/macosx/SystemOverview/Performance/
Performance.pdf>
I filed bug 80329 on the i18n converters, which have huge amounts of non-const 
global data.
Depends on: 80329
No longer depends on: 80329
Depends on: 80329
Sorry to say, but the patch did not compile under Linux.  Failed at line 62,
where it passed kCSSRawProperties into Init.
Hmm, probably need some const-dust sprinkled around.
Changing platform
Hardware: All → Macintosh
OS: Mac System 8.5 → All
Hardware: Macintosh → All
reassign all kandrot xpcom bug.
Assignee: kandrot → dougt
Blocks: 98284
Assignee: dougt → cathleen
performance bug.  dp, cathleen, you might want to try to cash in on this.
Actually, scc would be a better owner for this bug.

Scott: does this fix apply to all platforms or is it only for OS-X? The URL 
mentioned by sfraser has moved to:
http://developer.apple.com/techpubs/macosx/Essentials/Performance/Performance.pdf
The relevant info is on page 118 and 121.

If it's XP, the patch need to be changed or ifdef'd to compile under Linux and 
possibly other platforms.
Assignee: cathleen → scc
This is XP. Does it not compile on Linux?
Edward Kandrot [2001-05-14 15:06] says it doesn't.
Reclaim.

Does anyone know any reasons why the nsModuleComponentInfo of every module cannot 
be |const| (with a fix in nsIGenericFactory.h)?
Assignee: scc → sfraser
Makes perfect sense to me, although there might be modules doing weird things.
I agree. This should be const.
Mozilla builds with these conts changes. When this goes in, the components
array of individual modules can be made const one at a time.
Attachment #65333 - Attachment is obsolete: true
Comment on attachment 66205 [details] [diff] [review]
Spreading the const in ns{I}GenericFactory
[Checkin: Comment 19]

r=dp
Attachment #66205 - Flags: review+
Comment on attachment 66205 [details] [diff] [review]
Spreading the const in ns{I}GenericFactory
[Checkin: Comment 19]

sr=waterson
Attachment #66205 - Flags: superreview+
Comment on attachment 66205 [details] [diff] [review]
Spreading the const in ns{I}GenericFactory
[Checkin: Comment 19]

Patch checked in. Keeping bug open for more constness.
Attachment #66205 - Attachment is obsolete: true
Results of making gComponents const in libgkcontent.dylib (using pagestuff -p)
Before:

    MP_SECTION (__DATA,__data)
	size = 17340
    MP_SECTION (__DATA,__const)
	size = 130296

After:
    MP_SECTION (__DATA,__data)
	size = 13980
    MP_SECTION (__DATA,__const)
	size = 133656

so this moved ~3.3K of data to the const (i.e. paged once) section.
I checked in changes to make every 'static nsModuleComponentInfo' in the tree
|const|.
Status: NEW → ASSIGNED
Ongoing work, so Mozilla 1.2
Target Milestone: --- → mozilla1.2
Could the checkin from this bug be the cause why static build on Linux does no
longer work?

../../dist/lib/components/libucvcn.a(nsUCvCnModule.o): In function
`nsUnicodeToCP936Constructor(nsISupports *, nsID const &, void **)':
/home/kaie/current-trunk/mozilla-static/intl/uconv/ucvcn/../../../../mozilla/intl/uconv/ucvcn/nsUnicodeToCP936.h:60:
undefined reference to `nsUnicodeToCP936 virtual table'
../../dist/lib/components/libucvcn.a(nsUCvCnModule.o): In function
`nsUnicodeToCP936Constructor(nsISupports *, nsID const &, void **)':
/home/kaie/current-trunk/mozilla-static/intl/uconv/ucvcn/../../../../mozilla/intl/uconv/ucvcn/nsUCvCnModule.cpp:98:
undefined reference to `nsUnicodeToCP936 virtual table'
../../dist/lib/components/libucvlatin.a(nsUCvLatinModule.o): In function
`nsUnicodeToUTF16Constructor(nsISupports *, nsID const &, void **)':
/home/kaie/current-trunk/mozilla-static/intl/uconv/ucvlatin/../../../../mozilla/intl/uconv/ucvlatin/nsUnicodeToUCS2BE.h:117:
undefined reference to `nsUnicodeToUTF16 virtual table'

...
I watched the worms tinderbox, which is doing a static build, and it is still 
green.

> undefined reference to `nsUnicodeToCP936 virtual table'

This does not sound related.
This checkin is causing beos bustage.

c++  -frtti -fno-exceptions -Wall -Wconversion -Wpointer-arith
-Wbad-function-cast -Wcast-align -Woverloaded-virtual -Wsynth
-Wno-ctor-dtor-privacy -Wno-multichar -pedantic -Wno-long-long -pipe  -DNDEBUG
-DTRIMMED -O -nostart -Wl,-h -Wl,libnecko.so -o libnecko.so  nsNetModule.o     
 -Wl,--whole-archive ../../dist/lib/libneckobase_s.a
../../dist/lib/libneckodns_s.a ../../dist/lib/libneckosocket_s.a
../../dist/lib/libnkconv_s.a ../../dist/lib/libnkcnvts_s.a
../../dist/lib/libnkmime_s.a ../../dist/lib/libnkhttp_s.a
../../dist/lib/libnkfile_s.a ../../dist/lib/libnkdata_s.a
../../dist/lib/libnkjar_s.a ../../dist/lib/libnkres_s.a
../../dist/lib/libnkabout_s.a ../../dist/lib/libnkkwd_s.a 
-Wl,--no-whole-archive -L../../dist/bin -L../../dist/lib  -L../../dist/bin
-lxpcom -L../../dist/bin
-L/boot/home/tinderbox/BeOS_5.0_Depend/mozilla/obj-i686-pc-beos/dist/lib -lplds4
-lplc4 -lnspr4  ../../dist/lib/libunicharutil_s.a -lz    -lbe   
nsNetModule.o: In function `LC60':
nsNetModule.o(.data+0xb38): undefined reference to
`nsIndexedToHTML::Create(nsISupports *, nsID const &, void **)'
collect2: ld returned 1 exit status


If I remove the const from netwerk/build/nsNetModule.cpp's nsModuleComponentInfo
struct, then it links fine.  None of the other modules seem to have this problem.
What is it about nsIndexedToHTML::Create()?
It appears to be because nsIndexedToHTML::Create() is implemented in the header.
 If I move the implementation to nsIndexedToHTML.cpp and restore nsNetModule.cpp
then libnecko.so links fine.  Patch coming up.
Comment on attachment 67800 [details] [diff] [review]
Move nsIndexedToHTML::Create impl to .cpp
[Checkin: Comment 84]

sr=sfraser. Thanks for fixing this!
Attachment #67800 - Flags: superreview+
Need some const  loving here:

http://lxr.mozilla.org/seamonkey/source/layout/html/base/src/nsBulletFrame.cpp#525
Component: XPCOM → Layout
Target Milestone: mozilla1.2alpha → Future
The patch fixes nsStaticNameTable to take a const char* const [], and fixes the
places that use it to make their data tables const. These are kColorNames in
nsColorNames.cpp (GFX), kCSSRawProperties, kCSSRawKeywords, and a couple of
XSLT tables in unused code.
Attachment #29768 - Attachment is obsolete: true
Attachment #67800 - Attachment is obsolete: true
we need an automated way to find this stuff.. codesighs may be helpful here.
Yeah. What i've been doing is to dump the output of 'nm', and run it though a
perl script that demangles, and parses out class/method names and symbol types.
Comment on attachment 112574 [details] [diff] [review]
Make several static data tables const
[Checkin: Comment 53]

sr=alecf
Attachment #112574 - Flags: superreview+
Comment on attachment 112831 [details] [diff] [review]
Huge amount of const goodness in mozilla/intl

wow! this is fantastic
sr=alecf - can we get these in for 1.3? I'm really curious how our startup will
be improved, not to mention our footprint.
I'll try and get some codesighs numbers from my macho build after applying this.
Comment on attachment 112831 [details] [diff] [review]
Huge amount of const goodness in mozilla/intl

>Index: chardet/src/nsPSMDetectors.cpp
>-nsEUCStatistics *gZhTwStatisticsSet[ZHTW_DETECTOR_NUM_VERIFIERS] = {
>+const nsEUCStatistics* gZhTwStatisticsSet[ZHTW_DETECTOR_NUM_VERIFIERS] = {

How about |const nsEUCStatistics*const| (etc.) ?

That would require some of these things to be |const nsEUCStatistics*const*|:

>-nsXPCOMDetector::nsXPCOMDetector(PRUint8 aItems, nsVerifier **aVer, nsEUCStatistics** aStatisticsSet)
>+nsXPCOMDetector::nsXPCOMDetector(PRUint8 aItems, nsVerifier **aVer, const nsEUCStatistics** aStatisticsSet)


>Index: chardet/src/nsPSMDetectors.h
>-extern nsEUCStatistics *gZhTwStatisticsSet[];
>+extern const nsEUCStatistics *gZhTwStatisticsSet[];

ditto. (etc.)
In this patch I typedef'd nsVerifier and nsEUCStatistics to be const, and made
the arrays themselves const. I also made large amounts of data in the
nsFooVerifier.h headers const.
Attachment #112831 - Attachment is obsolete: true
Applied attachment 112834 [details] [diff] [review] to an opt macho build, and ran codesigh's basesummary
script before and after. Results:

Overall Change in Size
	Total:	      -7792 (+494392/-502184)
	Code:	     -19880 (+4596/-24476)
	Data:	     +12088 (+489796/-477708)

libuconv.dylib
	Total:	      -1440 (+478994/-480434)
	Code:	     -20568 (+3444/-24012)
	Data:	     +19128 (+475550/-456422)

libi18n.dylib
	Total:	      -6352 (+15398/-21750)
	Code:	       +688 (+1152/-464)
	Data:	      -7040 (+14246/-21286)

though I'm not sure how much I trust that data, since the size of
__mh_bundle_header apparently changed, and libuconv.dylib gained:
+19136	std::nothrow
in its S data section.
Comment on attachment 112834 [details] [diff] [review]
Follow dbaron's suggestion; yet more constness in mozilla/intl
[Checkin: Comment 54]

hey, works for me.
sr=alecf

I don't know whats up with that std::throw stuff, but I can't see this making
anything worse...:)
Attachment #112834 - Flags: superreview+
Attachment #112834 - Flags: review?(smontagu)
Comment on attachment 112834 [details] [diff] [review]
Follow dbaron's suggestion; yet more constness in mozilla/intl
[Checkin: Comment 54]

r=smontagu
Attachment #112834 - Flags: review?(smontagu) → review+
Comment on attachment 112834 [details] [diff] [review]
Follow dbaron's suggestion; yet more constness in mozilla/intl
[Checkin: Comment 54]

dbaron/simon - what are the chances we're going to get this into moz 1.3? Its
had review for a while now, we just need 1.3b approval...
Attachment #112834 - Flags: approval1.3b?
Comment on attachment 112574 [details] [diff] [review]
Make several static data tables const
[Checkin: Comment 53]

same thing here.. (also requesting approval)
Attachment #112574 - Flags: approval1.3b?
Fine by me  :)  I was waiting for 1.4 to open, but 1.3 will work.
Comment on attachment 112834 [details] [diff] [review]
Follow dbaron's suggestion; yet more constness in mozilla/intl
[Checkin: Comment 54]

We're already late and trying to get beta out ASAP. This is going to have to
wait.
Attachment #112834 - Flags: approval1.3b? → approval1.3b-
Attachment #112574 - Flags: approval1.3b? → approval1.3b-
Comment on attachment 114518 [details] [diff] [review]
Make kInlineFrameCID const
[Checkin: Comment 55]

sr=alecf
Attachment #114518 - Flags: superreview+
Comment on attachment 114701 [details] [diff] [review]
Fix nsMathMLContainerFrame not to rely on nsInlineFrame CID
[Checkin: Comment 56]

sr=alecf
Attachment #114701 - Flags: superreview+
Comment on attachment 112574 [details] [diff] [review]
Make several static data tables const
[Checkin: Comment 53]

Patch checked in.
Attachment #112574 - Attachment is obsolete: true
Comment on attachment 112834 [details] [diff] [review]
Follow dbaron's suggestion; yet more constness in mozilla/intl
[Checkin: Comment 54]

Patch checked in.
Attachment #112834 - Attachment is obsolete: true
Comment on attachment 114518 [details] [diff] [review]
Make kInlineFrameCID const
[Checkin: Comment 55]

Patch checked in.
Attachment #114518 - Attachment is obsolete: true
Comment on attachment 114701 [details] [diff] [review]
Fix nsMathMLContainerFrame not to rely on nsInlineFrame CID
[Checkin: Comment 56]

Patch checked in.
Keeping bug open for additional fixing.
Attachment #114701 - Attachment is obsolete: true
So... looking at the 21k static size increase for libuconv from this checkin,
this looks fishy:

      -124	g_ufJohabJamoMapping
     +21464	g_ufJohabJamoMapping

I suspect codesighs is just confused, since there was no real change here (nor
for nsColorNames::kColors, which supposedly grew by 1440 bytes....)
Yes, I think codesighs does a poor job of getting accurate symbol sizes. For
example, on Linux:

libuconv.so
    +478136 (+478204/-68)	R (DATA)
        +478136 (+478204/-68)	UNDEF:libuconv.so:R
             +48256	gGBKToUnicodeTable
...
    -456704 (+44/-456748)	D (DATA)
        -456704 (+44/-456748)	UNDEF:libuconv.so:D
             -48160	gGBKToUnicodeTable

but examination of the symbols (on OS X) shows that they are binary identical,
other than some padding at the end.

After my changes, on OS X, the lib actually got smaller:

-rwxr-xr-x    1 smfr  staff  4380320 Feb 26 09:38 libuconv_before.dylib
-rwxr-xr-x    1 smfr  staff  4379232 Feb 26 09:40 libuconv_after.dylib
Comment on attachment 115731 [details] [diff] [review]
Const changes in netwerk

>Index: base/src/nsIOService.cpp

>+const PRInt16 gBadPortList[] = { 

looks like this could be static as well.  otherwise, looks great!  r=darin
Attachment #115731 - Flags: review+
Comment on attachment 115701 [details] [diff] [review]
const goodness in htmlparser

kick ass!
though personally I prefer the C++ casting syntax:
PRUnichar('_')
r/sr=alecf
Attachment #115701 - Flags: superreview+
Comment on attachment 115715 [details] [diff] [review]
More const goodness in intl/uconv. Really.

r/sr=alecf (looks like a little overlap with the NS_DEFINE_IID work, but that's
fine)
Attachment #115715 - Flags: superreview+
Comment on attachment 115720 [details] [diff] [review]
Make nsModuleInfo const

nice! sr=alecf
Attachment #115720 - Flags: superreview+
Attachment #115720 - Flags: review?(dougt)
Comment on attachment 115720 [details] [diff] [review]
Make nsModuleInfo const

make sure that this doesn't break the static build. r=dougt.
Attachment #115720 - Flags: review?(dougt) → review+
Comment on attachment 115725 [details] [diff] [review]
Const changes in i18n
[Checkin: See comment 72+85]

r/sr=alecf
Attachment #115725 - Flags: superreview+
Comment on attachment 115731 [details] [diff] [review]
Const changes in netwerk

sr=alecf
I kind of feel like I've tried to make some PLDHashTableOps const before, and
it wouldn't build.. but if you've got it, then no complaints here :)
Attachment #115731 - Flags: superreview+
> I kind of feel like I've tried to make some PLDHashTableOps const before, and
> it wouldn't build.. but if you've got it, then no complaints here :)

You're right, it spits out warnings on Mac OS X. We should get pldhash to take a
const PLDHashTableOps*. I filed bug 195298.
Component: Layout → Layout: Misc Code
Comment on attachment 115725 [details] [diff] [review]
Const changes in i18n
[Checkin: See comment 72+85]

This was checked in.
Attachment #115725 - Attachment is obsolete: true
universalchardet had a ton of non-const data (about 10Kb worth) which really needs to be const.
Attachment #204418 - Flags: review?(bzbarsky)
Comment on attachment 115731 [details] [diff] [review]
Const changes in netwerk

This was never checked in  :(
Attachment #204419 - Flags: review?(brendan) → review+
Attachment #204418 - Flags: review?(bzbarsky) → review?(dbaron)
Comment on attachment 204418 [details] [diff] [review]
const goodness in extensions/universalchardet

>Index: src/base/JpCntx.cpp
>-        mRelSample[jp2CharContext[mLastCharOrder][order]]++;
>+        mRelSample[(int)jp2CharContext[mLastCharOrder][order]]++;

Construction-style cast would be much easier to read, i.e.:

mRelSample[int(p2CharContext[mLastCharOrder][order])]++

>Index: src/base/JpCntx.h
>-      mRelSample[jp2CharContext[mLastCharOrder][order]]++;
>+      mRelSample[(int)jp2CharContext[mLastCharOrder][order]]++;

ditto.

Not sure why you're casting, though.  Is it a warning fix?  Should the array be of unsigned char instead of char?

>Index: src/base/nsCodingStateMachine.h
> typedef struct 
> {
>-  nsPkgInt classTable;
>-  PRUint32 classFactor;
>-  nsPkgInt stateTable;
>-  const PRUint32* charLenTable;
>-  const char* name;
>+  const nsPkgInt  classTable;
>+  PRUint32        classFactor;
>+  const nsPkgInt  stateTable;

I don't see the point of the two consts added above.  Isn't that implied when an |SMModel| itself is const?

>+  const PRUint32* const charLenTable;

Likewise for the added second |const| on this line.

>+  const char*     name;
> } SMModel;

r=dbaron.  Sorry for the delay.
Attachment #204418 - Flags: review?(dbaron) → review+
Who is working on this? The patches look valid still (or at least large chunks of them)
Sorry, I'm not working on this any more.
Assignee: sfraser_bugs → nobody
Status: ASSIGNED → NEW
QA Contact: scc → layout.misc-code
Gary, are you able to test your patch applicability magic on this?

note: this is the last bug blocking  Bug 98284 xpcom footprint tracking
Keywords: helpwanted
(In reply to comment #75)
> Created an attachment (id=204419) [details]
> const goodness in xpconnect

Only attachment 204419 [details] [diff] [review] applies in mozilla-1.9.1 tree.

Status of all non-obsolete patches:
115701 - rotted
115715 - rotted
115720 - rotted
115731 - rotted
204418 - rotted
204419 - still applies
204420 - rotted

===

$ patch -p0 --dry-run < ~/Desktop/204419.txt 
patching file xpcvariant.cpp
Hunk #1 succeeded at 151 (offset 41 lines).
Keywords: checkin-needed
Severity: normal → trivial
Whiteboard: [c-n: xpconnect]
Target Milestone: Future → ---
Attachment #204419 - Attachment description: const goodness in xpconnect → const goodness in xpconnect [Checkin: Comment 82]
Attachment #204419 - Flags: approval1.9.1?
Keywords: checkin-needed
Whiteboard: [c-n: xpconnect] → [needs 1.9.1 landing: needs approval][c-n: xpconnect]
Attachment #204419 - Flags: approval1.9.1? → approval1.9.1-
Comment on attachment 204419 [details] [diff] [review]
const goodness in xpconnect
[Checkin: Comment 82]

Don't think we need this on branch.
Attachment #112574 - Attachment description: Make several static data tables const → Make several static data tables const [Checkin: Comment 53]
Attachment #112574 - Attachment is obsolete: false
Attachment #112834 - Attachment description: Follow dbaron's suggestion; yet more constness in mozilla/intl → Follow dbaron's suggestion; yet more constness in mozilla/intl [Checkin: Comment 54]
Attachment #112834 - Attachment is obsolete: false
Attachment #114518 - Attachment description: Make kInlineFrameCID const → Make kInlineFrameCID const [Checkin: Comment 55]
Attachment #114518 - Attachment is obsolete: false
Attachment #114701 - Attachment description: Fix nsMathMLContainerFrame not to rely on nsInlineFrame CID → Fix nsMathMLContainerFrame not to rely on nsInlineFrame CID [Checkin: Comment 56]
Attachment #114701 - Attachment is obsolete: false
Depends on: 207542
Attachment #66205 - Attachment description: Spreading the const in ns{I}GenericFactory → Spreading the const in ns{I}GenericFactory [Checkin: Comment 19]
Attachment #66205 - Attachment is obsolete: false
Attachment #67800 - Attachment description: Move nsIndexedToHTML::Create impl to .cpp → Move nsIndexedToHTML::Create impl to .cpp [Checkin: Comment 84]
Attachment #67800 - Attachment is obsolete: false
Comment on attachment 67800 [details] [diff] [review]
Move nsIndexedToHTML::Create impl to .cpp
[Checkin: Comment 84]


Move impl of nsIndexedToHTML::Create from .h to .cpp to fix BeOS bustage.
Bug 74803 sr=sfraser
2002-02-04 15:23	seawood%netscape.com 	mozilla/netwerk/streamconv/converters/nsIndexedToHTML.cpp 	1.19 	14/0
2002-02-04 15:23	seawood%netscape.com 	mozilla/netwerk/streamconv/converters/nsIndexedToHTML.h 	1.8 	1/12
Comment on attachment 115725 [details] [diff] [review]
Const changes in i18n
[Checkin: See comment 72+85]


Bug 207542 checked a part of this in,
but (the rest of) this patch was not actually checked in:
we should check whether we could do more here...
Attachment #115725 - Attachment description: Const changes in i18n → Const changes in i18n [Checkin: Comment 72+85]
Attachment #115725 - Attachment is obsolete: false
Attachment #115725 - Attachment description: Const changes in i18n [Checkin: Comment 72+85] → Const changes in i18n [Checkin: See comment 72+85]
Component: Layout: Misc Code → General
Flags: in-testsuite-
QA Contact: layout.misc-code → general
Whiteboard: [needs 1.9.1 landing: needs approval][c-n: xpconnect] → [ToDo: bitrotted patches, comment 85]
Target Milestone: --- → mozilla1.9.2a1
Whiteboard: [ToDo: bitrotted patches, comment 85] → [ToDo: bitrotted patches, comment 85] [good first bug]
Attachment #389757 - Flags: review?(dbaron)
Comment on attachment 389757 [details] [diff] [review]
const changes to extensions/spellcheck

r=dbaron, although it's probably worth fixing all 10 occurrences in http://mxr.mozilla.org/mozilla-central/search?string=static%20nsmodulecomponentinfo at once rather than doing it one-at-a-time
Attachment #389757 - Flags: review?(dbaron) → review+
Attachment #389757 - Attachment is obsolete: true
Attachment #389801 - Flags: review+
Comment on attachment 389801 [details] [diff] [review]
const changes for nsmodulecomponentinfo

>diff --git a/embedding/browser/photon/src/nsUnknownContentTypeHandler.cpp b/embedding/browser/photon/src/nsUnknownContentTypeHandler.cpp
>--- a/embedding/browser/photon/src/nsUnknownContentTypeHandler.cpp
>+++ b/embedding/browser/photon/src/nsUnknownContentTypeHandler.cpp
>@@ -199,17 +199,17 @@ NS_IMETHODIMP className::QueryInterface(
> 		else rv = NS_NOINTERFACE;
> 		} 
> 	return rv; 
> 	}
> 
> NS_GENERIC_FACTORY_CONSTRUCTOR( nsUnknownContentTypeHandler )
> 
> // The list of components we register
>-static nsModuleComponentInfo info[] = {
>+static const nsModuleComponentInfo info[] = {
> 		{
> 			"nsUnknownContentTypeHandler",
> 			NS_IHELPERAPPLAUNCHERDIALOG_IID,
> 			NS_IHELPERAPPLAUNCHERDLG_CONTRACTID,
> 			nsUnknownContentTypeHandlerConstructor
> 		}
> 	};
> 
>diff --git a/extensions/auth/nsAuthFactory.cpp b/extensions/auth/nsAuthFactory.cpp
>--- a/extensions/auth/nsAuthFactory.cpp
>+++ b/extensions/auth/nsAuthFactory.cpp
>@@ -204,17 +204,17 @@ NS_GENERIC_FACTORY_CONSTRUCTOR(nsAuthSSP
>   {0x93, 0x4b, 0x14, 0x8e, 0x33, 0xc2, 0x28, 0xa6} \
> }
> 
> #include "nsAuthSASL.h"
> NS_GENERIC_FACTORY_CONSTRUCTOR(nsAuthSASL)
> 
> //-----------------------------------------------------------------------------
> 
>-static nsModuleComponentInfo components[] = {
>+static const nsModuleComponentInfo components[] = {
>   { "nsAuthKerbGSS", 
>     NS_GSSAUTH_CID,
>     NS_AUTH_MODULE_CONTRACTID_PREFIX "kerb-gss",
>     nsKerbGSSAPIAuthConstructor
>   },
>   { "nsAuthNegoGSSAPI", 
>     NS_NEGOTIATEAUTH_CID,
>     NS_AUTH_MODULE_CONTRACTID_PREFIX "negotiate-gss",
>diff --git a/extensions/spellcheck/src/mozSpellCheckerFactory.cpp b/extensions/spellcheck/src/mozSpellCheckerFactory.cpp
>--- a/extensions/spellcheck/src/mozSpellCheckerFactory.cpp
>+++ b/extensions/spellcheck/src/mozSpellCheckerFactory.cpp
>@@ -112,17 +112,17 @@ mozInlineSpellCheckerConstructor(nsISupp
>   return rv;
> }
> 
> ////////////////////////////////////////////////////////////////////////
> // Define a table of CIDs implemented by this module along with other
> // information like the function to create an instance, contractid, and
> // class name.
> //
>-static nsModuleComponentInfo components[] = {
>+static const nsModuleComponentInfo components[] = {
> #ifdef MOZ_MACBROWSER
>     {
>         "OSX Spell check service",
>         MOZ_OSXSPELL_CID,
>         MOZ_OSXSPELL_CONTRACTID,
>         mozOSXSpellConstructor
>     },
> #else
>diff --git a/intl/build/nsI18nModule.cpp b/intl/build/nsI18nModule.cpp
>--- a/intl/build/nsI18nModule.cpp
>+++ b/intl/build/nsI18nModule.cpp
>@@ -51,17 +51,17 @@
> #include "nsStrBundleConstructors.h"
> 
> // locale
> #include "nsLocaleConstructors.h"
> 
> 
> NS_GENERIC_FACTORY_CONSTRUCTOR(nsSemanticUnitScanner)
> 
>-static nsModuleComponentInfo components[] =
>+static const nsModuleComponentInfo components[] =
> {
>  // lwbrk
>   { "Line Breaker", NS_LBRK_CID, 
>     NS_LBRK_CONTRACTID, nsJISx4051LineBreakerConstructor},
>   { "Word Breaker", NS_WBRK_CID,
>     NS_WBRK_CONTRACTID, nsSampleWordBreakerConstructor},
>   { "Semantic Unit Scanner", NS_SEMANTICUNITSCANNER_CID,
>     NS_SEMANTICUNITSCANNER_CONTRACTID, nsSemanticUnitScannerConstructor},
>diff --git a/intl/chardet/src/nsChardetModule.cpp b/intl/chardet/src/nsChardetModule.cpp
>--- a/intl/chardet/src/nsChardetModule.cpp
>+++ b/intl/chardet/src/nsChardetModule.cpp
>@@ -122,17 +122,17 @@ nsUKProbDetectorRegistrationProc(nsIComp
>                                  const nsModuleComponentInfo *info)
> {
>   return AddCategoryEntry(NS_CHARSET_DETECTOR_CATEGORY,
>                           "ukprob",
>                           info->mContractID);
> }
> 
> 
>-static nsModuleComponentInfo components[] =
>+static const nsModuleComponentInfo components[] =
> {
>  { "Meta Charset", NS_META_CHARSET_CID, 
>     NS_META_CHARSET_CONTRACTID, nsMetaCharsetObserverConstructor, 
>     nsMetaCharsetObserverRegistrationProc, nsMetaCharsetObserverUnegistrationProc,
>     NULL},
>  { "Document Charset Info", NS_DOCUMENTCHARSETINFO_CID, 
>     NS_DOCUMENTCHARSETINFO_CONTRACTID, nsDocumentCharsetInfoConstructor, 
>     NULL, NULL},
>diff --git a/modules/libjar/zipwriter/src/ZipWriterModule.cpp b/modules/libjar/zipwriter/src/ZipWriterModule.cpp
>--- a/modules/libjar/zipwriter/src/ZipWriterModule.cpp
>+++ b/modules/libjar/zipwriter/src/ZipWriterModule.cpp
>@@ -38,17 +38,17 @@
> 
> #include "nsIGenericFactory.h"
> #include "nsDeflateConverter.h"
> #include "nsZipWriter.h"
> 
> NS_GENERIC_FACTORY_CONSTRUCTOR(nsDeflateConverter)
> NS_GENERIC_FACTORY_CONSTRUCTOR(nsZipWriter)
> 
>-static nsModuleComponentInfo components[] =
>+static const nsModuleComponentInfo components[] =
> {
>   {
>     DEFLATECONVERTER_CLASSNAME,
>     DEFLATECONVERTER_CID,
>     "@mozilla.org/streamconv;1?from=uncompressed&to=deflate",
>     nsDeflateConverterConstructor,
>   },
>   {
>diff --git a/toolkit/xre/nsAppRunner.cpp b/toolkit/xre/nsAppRunner.cpp
>--- a/toolkit/xre/nsAppRunner.cpp
>+++ b/toolkit/xre/nsAppRunner.cpp
>@@ -952,17 +952,17 @@ ScopedXPCOMStartup::~ScopedXPCOMStartup(
>     mServiceManager = nsnull;
>   }
> }
> 
> // {95d89e3e-a169-41a3-8e56-719978e15b12}
> #define APPINFO_CID \
>   { 0x95d89e3e, 0xa169, 0x41a3, { 0x8e, 0x56, 0x71, 0x99, 0x78, 0xe1, 0x5b, 0x12 } }
> 
>-static nsModuleComponentInfo kComponents[] =
>+static const nsModuleComponentInfo kComponents[] =
> {
>   {
>     "nsXULAppInfo",
>     APPINFO_CID,
>     XULAPPINFO_SERVICE_CONTRACTID,
>     AppInfoConstructor
>   },
>   {
>diff --git a/widget/src/photon/nsWidgetFactory.cpp b/widget/src/photon/nsWidgetFactory.cpp
>--- a/widget/src/photon/nsWidgetFactory.cpp
>+++ b/widget/src/photon/nsWidgetFactory.cpp
>@@ -74,17 +74,17 @@ NS_GENERIC_FACTORY_CONSTRUCTOR(nsDragSer
> #endif
> NS_GENERIC_FACTORY_CONSTRUCTOR(nsSound)
> NS_GENERIC_FACTORY_CONSTRUCTOR(nsFilePicker)
> 
> #ifdef IBMBIDI
> NS_GENERIC_FACTORY_CONSTRUCTOR(nsBidiKeyboard)
> #endif
> 
>-static nsModuleComponentInfo components[] =
>+static const nsModuleComponentInfo components[] =
> {
>   { "Ph nsWindow",
>     NS_WINDOW_CID,
>     "@mozilla.org/widgets/window/ph;1",
>     nsWindowConstructor },
>   { "Ph Child nsWindow",
>     NS_CHILD_CID,
>     "@mozilla.org/widgets/child_window/ph;1",
Attachment #389801 - Flags: review+ → review?(dbaron)
Attachment #389801 - Attachment is obsolete: true
Attachment #389801 - Flags: review?(dbaron)
Attachment #389803 - Flags: review?(dbaron)
Comment on attachment 389803 [details] [diff] [review]
const changes for nsmodulecomponentinfo
[Checkin: Comment 92]

r=dbaron
Attachment #389803 - Attachment description: const changes for nsmodulecomponentinfo [UPDATED] → const changes for nsmodulecomponentinfo [UPDATED][checkin-needed]
Attachment #389803 - Flags: review?(dbaron) → review+
Comment on attachment 389803 [details] [diff] [review]
const changes for nsmodulecomponentinfo
[Checkin: Comment 92]


http://hg.mozilla.org/mozilla-central/rev/112b86848e92
Attachment #389803 - Attachment description: const changes for nsmodulecomponentinfo [UPDATED][checkin-needed] → const changes for nsmodulecomponentinfo [Checkin: Comment 92]
Would we any/all of these patches at this point, if a contributor could clean them up?
(In reply to Mike Hoye [:mhoye] from comment #93)
> Would we any/all of these patches at this point, if a contributor could
> clean them up?

Well, someone needs to first verify if comment 0 is (still) true.
Dropping good-first-bug status here, on the grounds that this need for this is not currently clear and we don't have a crisp description of a fix.
Whiteboard: [ToDo: bitrotted patches, comment 85] [good first bug] → [ToDo: bitrotted patches, comment 85]
You need to log in before you can comment on or make changes to this bug.