1151181 - uninitialized error string in mailnews/extensions/mdn/src/nsMsgMdnGenerator.cpp

Status: RESOLVED FIXED

(MailNews Core :: Composition, defect)

Description

[:aceman]

+++ This bug was initially created as a clone of Bug #1149320 comment 2 +++

Found by a subtle compile warning:
mailnews/extensions/mdn/src/nsMsgMdnGenerator.cpp:1133:83: warning: 'exitString' may be used uninitialized in this function [-Wmaybe-uninitialized]
     bundle->FormatStringFromName(exitString, params, 1, getter_Copies(failed_msg));

Comment 1

[:aceman]

 [e-mail] 2015-04-01 18:45:09 CEST

Comment on attachment 8585742 [details] [diff] [review] [diff] [review]
patch

>     switch (aExitCode)
>     {
>       case NS_ERROR_UNKNOWN_HOST:
>       case NS_ERROR_UNKNOWN_PROXY_HOST:
>         exitString = MOZ_UTF16("smtpSendFailedUnknownServer");
>         break;
>       case NS_ERROR_CONNECTION_REFUSED:
>@@ -1099,16 +1099,19 @@ NS_IMETHODIMP nsMsgMdnGenerator::OnStopR
>         exitString = MOZ_UTF16("smtpPasswordUndefined");
>         break;
>       default:
>         if (aExitCode != NS_ERROR_ABORT && !NS_IS_MSG_ERROR(aExitCode))
>           exitString = MOZ_UTF16("smtpSendFailedUnknownReason");
>       break;
>     }
As the compiler points out, the string is not set for NS_ERROR_ABORT or any NS_IS_MSG_ERROR error. Do we already have strings for those errors? Is there a function somewhere we can call to get the string name of those errors?

Comment 2

[:aceman]

:aceman 2015-04-04 10:55:21 CEST

(In reply to [e-mail] from comment #6)
> (In reply to aceman from comment #5)
> Aha, I found it.
> 
> It's called errorStringNameForErrorCode.
> 
> Sounds familiar to anyone?

Yeah, you seem to be right. Before bug 952493 the aExitCode was preserved in the "default" case and passed on to FormatStringFromID. After the bug, the exitString is left empty if the aExitCode is ABORT or one of the MSG errors. That is a regression.

Comment 3

[:aceman]

Attachment: patch

xpcshell tests passed for me locally. mozmill does not run as a whole so I can't say.

Comment 4

[[e-mail]]

Comment on attachment 8588288 [details] [diff] [review]
patch

>       case NS_ERROR_SMTP_PASSWORD_UNDEFINED:
>         exitString = MOZ_UTF16("smtpPasswordUndefined");
>         break;
>       default:
>-        if (aExitCode != NS_ERROR_ABORT && !NS_IS_MSG_ERROR(aExitCode))
>+        if (NS_IS_MSG_ERROR(aExitCode))
>+          exitString = errorStringNameForErrorCode(aExitCode);
>+        else
>           exitString = MOZ_UTF16("smtpSendFailedUnknownReason");
>       break;
Interestingly errorStringNameForErrorCode already defaults to smtpSendFailedUnknownReason, so you don't actually need to special-case it. It also knows about NS_ERROR_SMTP_PASSWORD_UNDEFINED so you can fold that case in too. (I don't have any better ideas for NS_ERROR_ABORT though.)

Comment 5

[:aceman]

Attachment: patch v2

Yes, good ideas.

Comment 6

[[e-mail]]

Comment on attachment 8588370 [details] [diff] [review]
patch v2

>       default:
>+        exitString = errorStringNameForErrorCode(aExitCode);
>-        if (aExitCode != NS_ERROR_ABORT && !NS_IS_MSG_ERROR(aExitCode))
>-          exitString = MOZ_UTF16("smtpSendFailedUnknownReason");
>-      break;
>     }
Please restore the break;

Comment 7

[:aceman]

What is the "break" good for in the default case?

Comment 8

[:aceman]

Attachment: patch v2.1

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

This patch has bitrotted, please update it (and set again the 'checkin-needed' keyword).

Comment 10

[:aceman]

Attachment: patch v2.2

Thanks, updated. Clash with Suyash's string fixes :) This is also an error string fix needed for TB38.

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/comm-central/rev/d41f334331bf

Comment 12

[Kent James (:rkent)]

Please set the target milestone to match the first revision where the patch landed, in this case Thunderbird 40. The affected flags are for other revisions. Yes it is badly misnamed.

Comment 13

[Kent James (:rkent)]

Comment on attachment 8592402 [details] [diff] [review]
patch v2.2

https://hg.mozilla.org/releases/comm-aurora/rev/47a68d5136bb
https://hg.mozilla.org/releases/comm-beta/rev/662ccf7959bc