Compiler warning: possibly uninitialized |rv| - mailnews/base/src/nsMsgPrintEngine.cpp

RESOLVED FIXED in Thunderbird 46.0

Status

Thunderbird
General
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: ISHIKAWA, Chiaki, Assigned: ISHIKAWA, Chiaki)

Tracking

unspecified
Thunderbird 46.0

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

2 years ago
Created attachment 8706430 [details] [diff] [review]
warning-nsMsgPrintEngine.patch

During local testing of my new patches for C-C TB to make sure that my
patches do not introduce new warnings (at least under linux where I
compile and test), I noticed a compiler warning from
nsMsgPrintEngine.cpp.

As I looked, |rv| ought to be initialized during normal processing.
But the logic seems a little twisted (see the comment in the code),
and obscure.

So even if |rv| may be properly initialized during normal processing, try telling it to the compiler and future community maintainers.

I simply set a default error value and added an error case handling
although the error ought to be rare. (The code is much saner with the |else| part.)

This shuts up the compiler warning (e.g., gcc 5.3).
(Assignee)

Updated

2 years ago
Attachment #8706430 - Flags: review?(bwinton)
(Assignee)

Updated

2 years ago
Assignee: nobody → ishikawa
Comment on attachment 8706430 [details] [diff] [review]
warning-nsMsgPrintEngine.patch

Review of attachment 8706430 [details] [diff] [review]:
-----------------------------------------------------------------

rs=me with that fixed.

::: mailnews/base/src/nsMsgPrintEngine.cpp
@@ +494,5 @@
>                             nullptr,                            // Referring URI
>                             nullptr,                            // Post data
>                             nullptr);                           // Extra headers
> +    else // we should report the error (!)
> +       rv = NS_ERROR_FAILURE;

If we've already initialized it above, I don't think we need to re-set it here…
Attachment #8706430 - Flags: review?(bwinton) → review+

Updated

2 years ago
Blocks: 1232107

Updated

2 years ago
Component: Untriaged → General

Updated

2 years ago
No longer blocks: 1232107
(Assignee)

Comment 2

2 years ago
Created attachment 8710690 [details] [diff] [review]
Revoming a compiler warning: possibly uninitialized |rv|.   carrying over r=bwinton

Updated patch. Carrying over r=bwinton. I will put checkin-needed keyword.

TIA
Attachment #8706430 - Attachment is obsolete: true
Attachment #8710690 - Flags: review+
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 3

2 years ago
https://hg.mozilla.org/comm-central/rev/9974e710d039
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 46.0
You need to log in before you can comment on or make changes to this bug.