Closed
Bug 127907
Opened 23 years ago
Closed 23 years ago
Remove dead string code
Categories
(Core :: XPCOM, defect, P2)
Tracking
()
RESOLVED
FIXED
mozilla1.0
People
(Reporter: alecf, Assigned: alecf)
Details
(Keywords: topembed+)
Attachments
(2 files)
39.80 KB,
patch
|
dp
:
review+
jag+mozilla
:
superreview+
asa
:
approval+
|
Details | Diff | Splinter Review |
51.25 KB,
patch
|
dp
:
review+
jag+mozilla
:
superreview+
asa
:
approval+
|
Details | Diff | Splinter Review |
So my little find-unused-exports tool has found two types of functions that we
can clean up:
1) singular uses of a particular string function. In almost every one of these
cases, the consumer is doing the wrong thing and ending up using some API they
shouldn't.
for example
nsString value(...);
value.StripChars(".");
this is lame because "." is a cstring, so they could be using NS_LITERAL_STRING,
but even worse is that this is a single-character string, so they could be saying
value.StripChar(PRUnichar('.'));
2) Methods that are simply never called.. for instance CompressSet() is never
called outside of nsString2.cpp
In an effort to continue cleaning up old string code, I've gone and cleaned up
the consumers of 1) above, and removed the relevant methods from 2)
Patch forthcoming.
Assignee | ||
Comment 1•23 years ago
|
||
I first tackled nsString.. reviews?
This will make it much easier to move the code to nsAString-based stuff,
because there will be less methods to migrate.
Assignee | ||
Comment 2•23 years ago
|
||
cc dp for reviews as well - this is a code space win, as we're removing lots of
dead code.
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: --- → mozilla1.0
Assignee | ||
Comment 4•23 years ago
|
||
jag: so does that mean you'll review this? :)
dp: can you r= this?
Assignee | ||
Comment 5•23 years ago
|
||
by the way, I've now tested this on windows, linux, and mac. No problems.
Comment 6•23 years ago
|
||
Comment on attachment 71539 [details] [diff] [review]
clean up nsString first
r=dp
Attachment #71539 -
Flags: review+
Comment 7•23 years ago
|
||
Comment on attachment 71539 [details] [diff] [review]
clean up nsString first
sr=jag
Attachment #71539 -
Flags: superreview+
Assignee | ||
Comment 8•23 years ago
|
||
Another patch, this time I cleaned up nsCString
Can I get another r=dp, sr=jag? Again, the patches are pretty mechanical.
Comment 9•23 years ago
|
||
Comment on attachment 72173 [details] [diff] [review]
now update nsCString
r=dp
Attachment #72173 -
Flags: review+
Comment 10•23 years ago
|
||
Comment on attachment 72173 [details] [diff] [review]
now update nsCString
When converting from |char| to |PRUnichar| and vice versa you need to do a
|unsigned char| cast in between, to protected against signed expansion where
|char| is signed. Perhaps we should put some helper functions up somewhere?
Other than that, sr=jag
Attachment #72173 -
Flags: superreview+
Comment 11•23 years ago
|
||
Comment on attachment 71539 [details] [diff] [review]
clean up nsString first
a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #71539 -
Flags: approval+
Comment 12•23 years ago
|
||
Comment on attachment 72173 [details] [diff] [review]
now update nsCString
a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #72173 -
Flags: approval+
Comment 13•23 years ago
|
||
nsbeta1- per ADT/Embed triage.
Assignee | ||
Comment 14•23 years ago
|
||
too late, it's been fixed.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Updated•4 years ago
|
Component: String → XPCOM
You need to log in
before you can comment on or make changes to this bug.
Description
•