482227 - add const to data tables in universalchardet

Status: RESOLVED FIXED

(Core :: Internationalization, defect)

Description

[Zeev Tarantov]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a1pre) Gecko/20090309 Minefield/3.2a1pre
Build Identifier: 

There are many KB of stuff in the shared object / dll that should be in read-only pages rather than in the writable data pages.

Much kudos to Diego E. 'Flameeyes' Pettenò (http://blog.flameeyes.eu/) for cowstats (https://www.ohloh.net/p/ruby-elf), it showed the objects that needed attention.

Reproducible: Always

Steps to Reproduce:
1. build universalchardet
2.
3.
Actual Results:  
lots of data pages

Expected Results:  
The read only data is marked read only to save memory and dynamic loading time. And because they taught us to use "const" in school.

Comment 1

[Zeev Tarantov]

Attachment: added const to the readonly data

against mozilla-central, tested on linux x86, but I really don't see this failing for any reason like compiler or architecture.

Comment 2

[Jo Hermans]

It was first filed as bug 74803, many eons ago ... Note also bug 201361.

Comment 3

[Zeev Tarantov]

Thank you for the links. I did not find the universal charset detector component in bugzilla.
I realize I am not the first person who has both 1) ever heard of "const" and 2) wanted to contribute to mozilla.
But the fact remains: as far as I can tell the patch in bug 201361, though it is marked fixed, has not been committed. Anyway, more writable data has been added since six years ago. The patches in bug 74803 have not been committed either. Four years ago, they were reviewed and approved, and two years ago someone said they are not working on this anymore.
Knowing the bug number for the last time someone was foolish enough to try to do some good is not a substitute for reviewing/testing/committing a working patch against current trunk repository. Or do I need to be friends with MoCo employees to contribute? Or maybe just switch to chrome...

Comment 4

[Jo Hermans]

No, that wasn't a remark for you - bug 74803 is still open, despite that there is a patch. And bug 201361 fixed some of the problems but not all (or someone repeated the same mistake again after it was fixed). Anyway, automated tests like cowstats should guard against these mistakes.

Comment 5

[Simon Montagu :smontagu]

As a rule, you should request review from somebody, as described in https://developer.mozilla.org/en/Hacking_Mozilla.

Thanks for the patch, checked in as http://hg.mozilla.org/mozilla-central/rev/c8ad626fd5e6

Comment 6

[Aiko]

Nothing left to do here, right?