1361647 - Coverity report: nsMathMLmtableFrame::​nsMathMLmtableFrame(nsStyleContext *): A scalar field is not initialized by the constructor

Status: RESOLVED FIXED

(Core :: MathML, defect, P3)

Description

[Mats Palmgren (inactive)]

Coverity CID 1225549 Uninitialized scalar field

The field will contain an arbitrary value left over from earlier computations.

In nsMathMLmtableFrame::​nsMathMLmtableFrame(nsStyleContext *): A scalar field is not initialized by the constructor 



159protected:
160  explicit nsMathMLmtableFrame(nsStyleContext* aContext)
161    : nsTableFrame(aContext)
   2. uninit_member: Non-static class member mFrameSpacingX is not initialized in this constructor nor in any functions that it calls.
   4. uninit_member: Non-static class member mFrameSpacingY is not initialized in this constructor nor in any functions that it calls.
   CID 1225549 (#1 of 1): Uninitialized scalar field (UNINIT_CTOR)6. uninit_member: Non-static class member mUseCSSSpacing is not initialized in this constructor nor in any functions that it calls.
162  {}

Comment 1

[Frédéric Wang (:fredw)]

Attachment: Patch

Comment 2

[Karl Tomlinson (:karlt)]

Comment on attachment 8865109 [details] [diff] [review]
Patch

I have always preferred to initialize variables only once when that is practical.
In the constructor here, no attempt is made to set these to the correct values and so they need to be set again later.

They are not used before set correctly, but setting incorrect values before the correct values are calculated can hide from tools such as valgrind a failure to set correct values.

Perhaps it is possible to call MapAllAttributesIntoCSS() from the constructor.  I don't know, but I also don't know why it is important to do so.

Perhaps Mats knows why Coverity thinks these should be set in the constructor.

Comment 3

[Mats Palmgren (inactive)]

> Perhaps Mats knows why Coverity thinks these should be set in the constructor.

Because it's a footgun to have uninitialized members laying around.

Comment 4

[Mats Palmgren (inactive)]

Comment on attachment 8865109 [details] [diff] [review]
Patch

LGTM, thanks!

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/84a52fd86062
Coverity report: nsMathMLmtableFrame::nsMathMLmtableFrame(nsStyleContext *): A scalar field is not initialized by the constructor. r=karlt

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/84a52fd86062