Closed Bug 467965 Opened 13 years ago Closed 13 years ago

Improve some of the string interfaces on nsISmtpServer

Categories

(MailNews Core :: Networking, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.0b2

People

(Reporter: standard8, Assigned: standard8)

References

Details

Attachments

(1 file)

Attached patch The fixSplinter Review
I'm doing these so we've got a slightly saner implementation for when I alter stuff for the password manager switch.

This changes some of the string interfaces from string to ACString. Not going with AUTF8String at this stage, as that will adjust the current functionality which I don't want to do without specific testing for l18n issues.
Attachment #351428 - Flags: superreview?(neil)
Attachment #351428 - Flags: review?(neil)
Comment on attachment 351428 [details] [diff] [review]
The fix

>-        if (*aUsername && **aUsername)
>-            return rv;
>-
>-        // empty username
>-        NS_Free(*aUsername);
>-        *aUsername = 0;
>+        if (aUsername.IsEmpty())
>+          return rv;
!aUsername.IsEmpty() surely?

>+  return mPrefBranch->SetCharPref("hostname", nsCString(aHostname).get());
Nit: please use PromiseFlatCString (2×).

>+    uri.AssignLiteral("smtp");
>+    uri.AppendLiteral("://");
AssignLiteral("smtp://");

Should this need to support smtps:// in the future, I think we can live with an if choosing between the two literals ;-)
Attachment #351428 - Flags: superreview?(neil)
Attachment #351428 - Flags: superreview+
Attachment #351428 - Flags: review?(neil)
Attachment #351428 - Flags: review+
Checked in with comments addressed. http://hg.mozilla.org/comm-central/rev/1e7d1a981aa2
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
I had to revert some of the ways we return values from the Set* functions that I altered; changeset: http://hg.mozilla.org/comm-central/rev/89b309ce721c

I also added a comment as to why.
You need to log in before you can comment on or make changes to this bug.