I think we should add -Wwrite-strings to the compiler options we use for gcc. The gcc info page describes it as: `-Wwrite-strings' When compiling C, give string constants the type `const char[LENGTH]' so that copying the address of one into a non-`const' `char *' pointer will get a warning; when compiling C++, warn about the deprecated conversion from string constants to `char *'. These warnings will help you find at compile time code that can try to write into a string constant, but only if you have been very careful about using `const' in declarations and prototypes. Otherwise, it will just be a nuisance; this is why we did not make `-Wall' request these warnings. However, I think we should try to reduce the number of warnings in current builds *first*. I've been building with it for a few weeks, and it's not too noisy, but there are some files that yield a bunch of warnings. We also need to check if this option exists on all the versions of gcc that we support.
I like this a lot. I personally would say we should just turn this on now - I don't see a reason to wait - the more warnings we make visible, the sooner people will fix them (and it distributes the load so myself or dbaron aren't cleaning them all up)
Note that anyone who actually writes to these strings is going to cause a segfault anyway, so I'd be surprised if this was a big issue.
I rebuilt my 5-10 tree with -Wwrite-strings and there were 2063 additional warnings, 1494 from C++ files and the rest from C files. Many of the C++ files had multiple warnings. Here are the top ten: 20 check_off.xpm:22 21 check_on.xpm:23 40 gtkmozembed2.cpp:162 49 nsXIContext.cpp:196 58 nsXPLookAndFeel.cpp:189 60 nsInstallResources.cpp:76 96 nsObjectFrame.cpp:2729 124 nsGtkMenu.cpp:164 305 splash.xpm:307 321 logo.xpm:323 That's an awful lot of noise. The C warnings are much more scattered. It seems to me that you would get more useful results by adding -O so gcc will do data flow analysis.
Roughly a third (including the top two offenders) are from xpm files. Those are generated by various programs, so we can't really do anything about them directly. The next entry comes from GtkFactoryEntry, which is a struct we don't define, with gchar* members, not const gchar*. GTK2 is the same. I don't know if these are modifyable under some circumstances. I guess that this is why the warning isn't on by default...
Assignee: dbaron → nobody
Component: Build Config → Build Config
Product: Mozilla Application Suite → Core
QA Contact: granrosebugs → build-config
build/moz.configure/warnings.configure is setting -Wwrite-strings. Not sure when it was introduced. But this bug is definitely implemented.
Status: NEW → RESOLVED
Last Resolved: 11 months ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.