Closed Bug 1373232 Opened 3 years ago Closed 3 years ago

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

Categories

(Core :: Layout, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: andi, Assigned: andi)

References

(Blocks 1 open bug)

Details

(Keywords: coverity, Whiteboard: CID 1412727, CID 1412726)

Attachments

(1 file, 1 obsolete file)

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)
>>    };
Attachment #8878014 - Attachment is obsolete: true
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 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
Attachment #8878015 - Flags: review?(mats) → review+
Pushed by bpostelnicu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/84a58eb9d35b
Use floating point division when dividing nscoord values with a twipsPerPixel factor. r=mats
https://hg.mozilla.org/mozilla-central/rev/84a58eb9d35b
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.