Closed Bug 974785 Opened 6 years ago Closed 6 years ago

netwerk/protocol/http/nsHttpConnection.cpp:351:9 [-Wsometimes-uninitialized] variable 'rv' is used uninitialized

Categories

(Core :: Networking: HTTP, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla30
Tracking Status
firefox29 --- unaffected
firefox30 --- fixed

People

(Reporter: cpeterson, Assigned: cpeterson)

References

(Blocks 1 open bug)

Details

(Keywords: regression)

Attachments

(1 file)

These clang warnings are a regression from bug 970939:

netwerk/protocol/http/nsHttpConnection.cpp:351:9 [-Wsometimes-uninitialized] variable 'rv' is used uninitialized whenever 'if' condition is false

netwerk/protocol/http/nsHttpConnection.cpp:351:9 [-Wsometimes-uninitialized] variable 'rv' is used uninitialized whenever '&&' condition is false



nsresult rv won't be initialized at the NS_SUCCEEDED check at [1] if we don't take the code path through [2]. That NS_SUCCEEDED used to check the return from `rv = OnOutputStreamReady(mSocketOut)` until bug 970939 rearranged the code.

[1] https://hg.mozilla.org/mozilla-central/annotate/660b62608951/netwerk/protocol/http/nsHttpConnection.cpp#l367

[2] https://hg.mozilla.org/mozilla-central/annotate/660b62608951/netwerk/protocol/http/nsHttpConnection.cpp#l352
Attachment #8378817 - Flags: review?(mcmanus)
Comment on attachment 8378817 [details] [diff] [review]
fix-uninitialized-warning.patch

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

steve can review this
Attachment #8378817 - Flags: review?(mcmanus) → review?(sworkman)
Comment on attachment 8378817 [details] [diff] [review]
fix-uninitialized-warning.patch

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

Ah, oops. This looks fine to me. Thanks for the patch.

I'd like to add one thing though - could you move the declaration of 'nsresult rv' to line 351, between the comment and the if statement. Just to be clearer about the first use of rv. Also, init it to NS_OK.

::: netwerk/protocol/http/nsHttpConnection.cpp
@@ +366,5 @@
>  
> +    rv = StartShortLivedTCPKeepalives();
> +    if (NS_WARN_IF(NS_FAILED(rv))) {
> +        LOG(("nsHttpConnection::Activate [%p] "
> +             "StartShortLivedTCPKeepalives failed rv2[0x%x]",

s/rv2/rv/
Attachment #8378817 - Flags: review?(sworkman) → review+
Landed with requested changes: moved `nsresult rv = NS_OK` definition and s/rv2/rv/

https://hg.mozilla.org/integration/mozilla-inbound/rev/1fbf700a1ba6
https://hg.mozilla.org/mozilla-central/rev/1fbf700a1ba6
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.