Closed Bug 368724 Opened 13 years ago Closed 13 years ago

Clean up tzid comparison/upgrade code


(Calendar :: Internal Components, defect)

Sunbird 0.3
Not set


(Not tracked)

Sunbird 0.3


(Reporter: mattwillis, Assigned: mattwillis)


(Whiteboard: [verified0.3.1])


(1 file)

In bug 368121, mvl found these issues with post-review and checkin:

>>Index: calendar/base/src/calICSService.cpp
>>+calICSService::LatestTzId(const nsACString& tzid, nsACString& _retval) {
>>+    // If it doesn't start with "/" then it isn't ours.
>>+    if (!StringBeginsWith(
>>+            tzid, nsDependentCString(STRLEN_ARGS("/")))) {
>>+        return NS_OK;
>You must assign something to _retval here. Otherwise, it will be undefined.
>That might be ok in case of an error, but you return a success code. That means
>the return value must have some meaning.

>>+    if (continent == NS_LITERAL_CSTRING("Africa")) {
>I wonder if |continent.EqualsLiteral("Africa")| would be any faster. At least
>it would be more readable, due to less uppercase shouting.
>Same goes for all the following string comparisons.

>>+    if ((_retval.Length() == 0) &&
>You don't know what _retval was, so you can't assume it's empty. You never
>initialized it.

These should be addressed before 0.3.1
Flags: blocking-calendar0.3.1+
Attachment #253375 - Flags: second-review?(mvl)
Attachment #253375 - Flags: first-review?(ctalbert.moz)
OS: AIX → All
Comment on attachment 253375 [details] [diff] [review]
Addresses mvl's comments

Tested and built on MacOS X. Looks like it does a good job of addressing the comments and still works.
Attachment #253375 - Flags: first-review?(ctalbert.moz) → first-review+
Comment on attachment 253375 [details] [diff] [review]
Addresses mvl's comments

Looks good, except prefer |_retval.Truncate()| to  assigning in "".  r2=dmose with that change.
Attachment #253375 - Flags: second-review?(mvl) → second-review+
Patch (with nit) checked in on SUNBIRD_0_3_BRANCH, LIGHTNING_0_3_BRANCH, MOZILLA_1_8_BRANCH, and trunk.

Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [fixed0.3.1]
VERIFIED on 0.3.1 (Mac OS X)
Lightning: 2007020304 (Thunderbird: version 2 beta 2
Sunbird: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O;
en-US; rv:1.9a1) Gecko/20070203 Sunbird/0.3.1

(needs Verifying on 0.5)
Whiteboard: [fixed0.3.1] → [verified0.3.1]
You need to log in before you can comment on or make changes to this bug.