Closed
Bug 686188
Opened 13 years ago
Closed 13 years ago
Replace nsCStringArray in news with nsTArray counterparts
Categories
(MailNews Core :: Networking: NNTP, defect)
MailNews Core
Networking: NNTP
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 10.0
People
(Reporter: standard8, Assigned: standard8)
References
Details
Attachments
(1 file, 1 obsolete file)
10.69 KB,
patch
|
Bienvenu
:
review+
|
Details | Diff | Splinter Review |
nsCStringArray is getting old and obsolete, this helps removing it get a bit further along.
Attachment #559737 -
Flags: review?(dbienvenu)
Assignee | ||
Comment 1•13 years ago
|
||
I'm still working on fixing the other one, but this one should work fine.
Comment 2•13 years ago
|
||
Comment on attachment 559737 [details] [diff] [review] The fix r=me, but no need for braces here: + for (PRUint32 i = 0; i < length; ++i) + { + SetAsSubscribed(mSubscribedNewsgroups[i]); + }
Attachment #559737 -
Flags: review?(dbienvenu) → review+
Assignee | ||
Comment 3•13 years ago
|
||
Ok, this is updated for the PRBool changes, and I also made the AppendIfSearchMatch inline to get rid of the extra function.
Attachment #559737 -
Attachment is obsolete: true
Attachment #564792 -
Flags: review?(dbienvenu)
Comment 4•13 years ago
|
||
Comment on attachment 564792 [details] [diff] [review] The fix v2 + hostInfoStream->Write(mGroupsOnServer[i].get(), mGroupsOnServer[i].Length(), + &bytesWritten); bytesWritten could be indented more to line up with mGroupsOnServer.
Attachment #564792 -
Flags: review?(dbienvenu) → review+
Assignee | ||
Comment 5•13 years ago
|
||
Checked in: http://hg.mozilla.org/comm-central/rev/7767f7386c9d
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 10.0
Comment 6•13 years ago
|
||
Comment on attachment 564792 [details] [diff] [review] The fix v2 Review of attachment 564792 [details] [diff] [review]: ----------------------------------------------------------------- ::: mailnews/news/src/nsNntpIncomingServer.cpp @@ -1916,4 +1880,4 @@ > > if (colID[0] == 'n') { > > nsCAutoString str; > > if (mSearchResultSortDescending) > > - row = mSubscribeSearchResult.Count() + ~row; > > + row = mSubscribeSearchResult.Length() + ~row; nsCStringArray::Count() returns a PRInt32. nsTArray::Length() returns a PRUint32. This, I suspect, is what caused bug 693365. (~row = -row - 1 = -(row + 1)). I think this (and a few other places we play the ~row trick) should be fixed to use - (row + 1), since it's clearer and any decent optimizer could make the two forms equivalent. I just confirmed that gcc -O3 is smart enough to do this.
Comment 7•13 years ago
|
||
Comment on attachment 564792 [details] [diff] [review] The fix v2 > nsCAutoString str; > if (mSearchResultSortDescending) >- row = mSubscribeSearchResult.Count() + ~row; >- mSubscribeSearchResult.CStringAt(row, str); >+ row = mSubscribeSearchResult.Length() + ~row; >+ mSubscribeSearchResult.SafeElementAt(row, str); > // some servers have newsgroup names that are non ASCII. we store > // those as escaped. unescape here so the UI is consistent > rv = NS_MsgDecodeUnescapeURLPath(str, _retval); So near, and yet so far... +~row compiles to the same code in signed and unsigned arithmetic, although that doesn't make it readable. The actual error is that the weird CStringAt function was not translated correctly, it should be something like if (mSearchResultSortDescending) row = mSubscribeSearchResult.Length() - 1 - row; rv = NS_MsgDecodeUnescapeURLPath(mSubscribeSearchResult.ElementAt(row), _retval);
You need to log in
before you can comment on or make changes to this bug.
Description
•