1373232 - [Static Analysis] Result is not floating point in nsCSSRendering.cpp

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Andi [:andi]]

The Static Analysis tool Coverity detected that in several places the results of different expressions is not correctly calculated as float:

(1)
>>  Float spreadDistance = shadowItem->mSpread / twipsPerPixel;

(2)
>>  Float outlineWidths[4] = { Float(width / twipsPerPixel),
>>                             Float(width / twipsPerPixel),
>>                             Float(width / twipsPerPixel),
>>                             Float(width / twipsPerPixel) };

I think the same reasoning can be applied here:
(3)
>>  // convert the border widths
>>  Float outlineWidths[4] = { Float(width / twipsPerPixel),
>>                             Float(width / twipsPerPixel),
>>                             Float(width / twipsPerPixel),
>>                             Float(width / twipsPerPixel) };

(4)
>>    Float borderSizes[4] = {
>>      Float(border.top / twipsPerPixel),
>>      Float(border.right / twipsPerPixel),
>>      Float(border.bottom / twipsPerPixel),
>>      Float(border.left / twipsPerPixel)
>>    };

Comment 1

[Andi [:andi]]

Attachment: vscode setup

* * *
[mq]: 2017-06-06_18-20-49_r362472+.diff

* * *
[mq]: 2017-06-08_13-14-43_r362472+.diff

* * *
[mq]: 2017-06-13_11-34-02_r363414+.diff

* * *
[mq]: 2017-06-13_16-53-50_r363414+.diff

Review commit: https://reviewboard.mozilla.org/r/149428/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/149428/

Comment 2

[Andi [:andi]]

Attachment: Bug 1373232 - Use floating point division when dividing nscoord values with a twipsPerPixel factor.

Review commit: https://reviewboard.mozilla.org/r/149430/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/149430/

Comment 3

[Andi [:andi]]

Comment on attachment 8878015 [details]
Bug 1373232 - Use floating point division when dividing nscoord values with a twipsPerPixel factor.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/149430/diff/1-2/

Comment 4

[Mats Palmgren (inactive)]

Fwiw, static_cast<Float> of a nscoord seems a bit strange to me.
Can we just use Float() there for consistency with the other places please?

nit on the commit message:
> ensure result of expression is flaoting-point

What you're really doing is ensuring that the _division_ is floating point.
So I would probably write something like:

"Use floating point division when dividing nscoord values with a twipsPerPixel factor."

Comment 5

[Andi [:andi]]

Comment on attachment 8878015 [details]
Bug 1373232 - Use floating point division when dividing nscoord values with a twipsPerPixel factor.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/149430/diff/2-3/

Comment 6

[Mats Palmgren (inactive)]

Comment on attachment 8878015 [details]
Bug 1373232 - Use floating point division when dividing nscoord values with a twipsPerPixel factor.

https://reviewboard.mozilla.org/r/149430/#review154800

Looks good, thanks.  r=mats

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/84a58eb9d35b
Use floating point division when dividing nscoord values with a twipsPerPixel factor. r=mats

Comment 8

[Carsten Book [:Tomcat]]

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