Closed Bug 1145050 Opened 5 years ago Closed 5 years ago

Coverity detected 8 uninitialized class members in the Http2Session::Http2Session constructor

Categories

(Core :: Networking: HTTP, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox39 --- fixed

People

(Reporter: bagder, Assigned: bagder)

References

(Blocks 1 open bug)

Details

(Keywords: coverity, Whiteboard: [spdy])

Attachments

(1 file, 1 obsolete file)

Coverity CID 1286435 

 uninit_member: Non-static class member mInputFrameDataSize is not initialized in this constructor nor in any functions that it calls.
 uninit_member: Non-static class member mInputFrameDataRead is not initialized in this constructor nor in any functions that it calls.
 uninit_member: Non-static class member mInputFrameType is not initialized in this constructor nor in any functions that it calls.
 uninit_member: Non-static class member mInputFrameFlags is not initialized in this constructor nor in any functions that it calls.
 uninit_member: Non-static class member mInputFrameID is not initialized in this constructor nor in any functions that it calls.
 uninit_member: Non-static class member mPaddingLength is not initialized in this constructor nor in any functions that it calls.
 uninit_member: Non-static class member mFlatHTTPResponseHeadersOut is not initialized in this constructor nor in any functions that it calls.
 uninit_member: Non-static class member mPreviousPingThreshold is not initialized in this constructor nor in any functions that it calls.
Comment on attachment 8579888 [details] [diff] [review]
0001-Bug-1145050-fix-uninitialized-class-members-in-Http2.patch

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

One change, otherwise looks good. r=me with that change.

::: netwerk/protocol/http/Http2Session.cpp
@@ +105,5 @@
>    , mOutputQueueUsed(0)
>    , mOutputQueueSent(0)
>    , mLastReadEpoch(PR_IntervalNow())
>    , mPingSentEpoch(0)
> +  , mPreviousPingThreshold(PR_IntervalNow())

This doesn't hurt anything (the default value will never be used in the code as it is), but I think I'd feel better if we just assigned mPreviousPingThreshold = mPingThreshold in the body of the ctor, just in case. (If we were to ever use mPreviousPingThreshold's default value, the one you're assigning there would likely be woefully wrong.)
Attachment #8579888 - Flags: review?(hurley) → review+
Whiteboard: [spdy]
Comment on attachment 8579888 [details] [diff] [review]
0001-Bug-1145050-fix-uninitialized-class-members-in-Http2.patch

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

I think this report is pretty much a false positive - the state machine should keep those things from being used uninitialized - but cleaning up false positives, so we can see true positives, is a worth the tiny overhead.

::: netwerk/protocol/http/Http2Session.cpp
@@ +105,5 @@
>    , mOutputQueueUsed(0)
>    , mOutputQueueSent(0)
>    , mLastReadEpoch(PR_IntervalNow())
>    , mPingSentEpoch(0)
> +  , mPreviousPingThreshold(PR_IntervalNow())

I agree with nick.. please make that change
Thanks both of you. Adjusted as requested. Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8cb404e52447
Attachment #8579888 - Attachment is obsolete: true
Attachment #8580555 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/1eb641d3b68e
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
You need to log in before you can comment on or make changes to this bug.