64016 - Remove nsCString::GetBuffer()

Status: RESOLVED FIXED

(Core :: XPCOM, defect, P3)

Description

[Peter "jag" Annema]

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.

Comment 1

[Peter "jag" Annema]

Attachment: [patch] step 1a, remove most GetBuffer clients (still some left)

Comment 2

[Peter "jag" Annema]

Attachment: [patch] step2: fix nsString.h and nsString2.cpp

Comment 3

[timeless]

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.

Comment 4

[Peter "jag" Annema]

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.

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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?

Comment 6

[Peter "jag" Annema]

Attachment: [patch] step1a-update1: fixes for comments timeless & dbaron

Comment 7

[Peter "jag" Annema]

| 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...

Comment 8

[Peter "jag" Annema]

Attachment: [patch] step1b: More GetBuffer -> get

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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.

Comment 10

[Peter "jag" Annema]

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.)

Comment 11

[Peter "jag" Annema]

Attachment: [patch] step1a-nsAddressBook.cpp

Comment 12

[Scott Collins]

I'd like to have all the |.get| changes in for mozilla0.9.

Comment 13

[Peter "jag" Annema]

Great :-) How best can we get that done? (Other than r=/a= grunt work).

Comment 14

[Peter "jag" Annema]

Okay, started from scratch with a new patch and way less noise.

Taking this bug from scc so I have it on my radar.

Comment 15

[Peter "jag" Annema]

Attachment: [patch] This should remove all nsCString::GetBuffer()

Comment 16

[Scott Collins]

builds on Mac :-)

Comment 17

[Peter "jag" Annema]

Attachment: [patch] update to nsLocaleService.cpp after recent checkin to it. Decided to clean up :-)

Comment 18

[timeless]

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?]

Comment 19

[Peter "jag" Annema]

> 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.

Comment 20

[Peter "jag" Annema]

Attachment: [patch] incorporated timeless' comments, synch'ing

Comment 21

[Peter "jag" Annema]

Patch mostly checked in. nsString.h change this weekend.

Comment 22

[Peter "jag" Annema]

*grmbl*

Thanks syd. Take all the fun out of it, why don't you ;-p

Marking this fixed.