Closed
Bug 64016
Opened 24 years ago
Closed 24 years ago
Remove nsCString::GetBuffer()
Categories
(Core :: XPCOM, defect, P3)
Core
XPCOM
Tracking
()
RESOLVED
FIXED
mozilla0.8.1
People
(Reporter: jag+mozbugs, Assigned: jag+mozbugs)
References
Details
Attachments
(8 files)
266.58 KB,
patch
|
Details | Diff | Splinter Review | |
1.71 KB,
patch
|
Details | Diff | Splinter Review | |
55.67 KB,
patch
|
Details | Diff | Splinter Review | |
53.38 KB,
patch
|
Details | Diff | Splinter Review | |
5.83 KB,
patch
|
Details | Diff | Splinter Review | |
233.35 KB,
patch
|
Details | Diff | Splinter Review | |
3.99 KB,
patch
|
Details | Diff | Splinter Review | |
236.20 KB,
patch
|
Details | Diff | Splinter Review |
GetBuffer() has been depricated in favor of get() to get more unity in code. This bug will be for tracking changing of all nsCString::GetBuffer() uses to nsCString::get() so GetBuffer can be removed.
Assignee | ||
Comment 1•24 years ago
|
||
Assignee | ||
Comment 2•24 years ago
|
||
do you seriously expect anyone to review this ;-? notes to self: write("T",1), += " ", non PR ints.. comparisons w/ nsnull, null, 0 .Length() > 0 initializers should be in the order of class definition General questions: title.AssignWithConversion("Error"); // localization rv = picker->SetDefaultString(NS_ConvertASCIItoUCS2(suggested.get()).GetUnicode() ); rv =a => rv = a I think there was only one rv =a, please fix that. Cleaning up the other nonsense is for someone else.
Assignee | ||
Comment 4•24 years ago
|
||
Well, I'm hoping someone will, seeing how I've kept this patch mostly to fixing |GetBuffer()| uses, and do explicit |get()| where needed (though not everywhere yet). If I'd add all that other fixing up I'm sure this will just gather dust.
Here are comments on the first part of the first patch, up to and including nsAbCard.cpp: nsLocationImpl.cpp: filed bug 64041 on other issues nsLocalService.cpp: should use langcstr.ToNewCString() (2 times) should untabify? nsDocumentEncoder.cpp: This code (around the first change) is rather evil. Maybe you should fix it, or at least file a bug on its owner?
Assignee | ||
Comment 6•24 years ago
|
||
Assignee | ||
Comment 7•24 years ago
|
||
| rv =a => rv = a | nsLocalService.cpp: | should use langcstr.ToNewCString() (2 times) | should untabify? Those are done, and some more strdup -> ToNewCString() . strlen -> Length() changes. | nsDocumentEncoder.cpp: | This code (around the first change) is rather evil. Maybe you should | fix it, or at least file a bug on its owner? I guess I'll file a bug...
Assignee | ||
Comment 8•24 years ago
|
||
r=dbaron on attachment 21491 [details] [diff] [review] up to and including nsImapIncomingServer.cpp, with the following comments: nsAddressBook.cpp: * should file bug on AddressBookParser::AddLdifColToDatabase to switch colType from nsCAutoString to nsLiteralCString and use new string tools in nsReadableUtils.h * Perhaps it would be better to rename the valueSlot parameter to column rather than changing all the uses. nsMsgThread.cpp: * Perhaps it would be better to reorder the initializers rather than the members themselves? It seems the members are more likely to be in the "intended" order.
Assignee | ||
Comment 10•24 years ago
|
||
It turns out someone else already checked in the change to nsMsgThread.cpp, but in general I think you're right that it's better to change the initializer order than the member order. Attaching a patch for the issues in nsAddressBook.cpp, will hopefully remember to file that new bug :-) (Perhaps turn it into a general clean-up one? Remove some CIDs, replace WITH_SERVICE macros, etc.)
Assignee | ||
Comment 11•24 years ago
|
||
Comment 12•24 years ago
|
||
I'd like to have all the |.get| changes in for mozilla0.9.
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9
Assignee | ||
Comment 13•24 years ago
|
||
Great :-) How best can we get that done? (Other than r=/a= grunt work).
Updated•24 years ago
|
Priority: -- → P3
Assignee | ||
Comment 14•24 years ago
|
||
Okay, started from scratch with a new patch and way less noise. Taking this bug from scc so I have it on my radar.
Assignee: scc → disttsc
Status: ASSIGNED → NEW
Assignee | ||
Comment 15•24 years ago
|
||
Comment 16•24 years ago
|
||
builds on Mac :-)
Assignee | ||
Comment 17•24 years ago
|
||
Comment 18•24 years ago
|
||
26508 r=timeless 26178 r=timeless w/ few requests NS_CONST_CAST(char*,var.get()),(char*)var.get() => var.get() new nsCString(var.get()) => var.ToNewCString() afaik .Append*(var.get) drop .get() [also for .Equals?]
Assignee | ||
Comment 19•24 years ago
|
||
> NS_CONST_CAST(char*,var.get()),(char*)var.get() => var.get() Specifically for |->SetSpec| on nsIURI. Done. timeless: some of those (char*) / NS_CONST_CAST(char*, ...) are needed for ugliness like nsUnescape. > new nsCString(var.get()) => var.ToNewCString() Actually |.ToNewString()|, but it just does |return new nsCString(var);|, so I'll just drop the |.get()|. Done. > afaik .Append*(var.get) drop .get() [also for .Equals?] Done. I've also removed |.get()| in |new nsCStringKey(var.get());| I'm building now with these changes, I'll attach the new patch when that compiles fine.
Assignee | ||
Comment 20•24 years ago
|
||
Assignee | ||
Comment 21•24 years ago
|
||
Patch mostly checked in. nsString.h change this weekend.
Updated•24 years ago
|
Target Milestone: mozilla0.9 → mozilla0.8.1
Assignee | ||
Comment 22•24 years ago
|
||
*grmbl* Thanks syd. Take all the fun out of it, why don't you ;-p Marking this fixed.
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Updated•3 years ago
|
Component: String → XPCOM
You need to log in
before you can comment on or make changes to this bug.
Description
•