[Static Analysis][Result is not floating-point] In function nestegg_duration

RESOLVED FIXED

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: andi, Assigned: kinetik)

Tracking

(Blocks: 1 bug, {coverity})

Trunk
coverity
Points:
---

Firefox Tracking Flags

(firefox50 affected)

Details

(Whiteboard: CID 1362113)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
The Static Analysis tool Coverity detected that result is not floating point in the following operation:

>>if (unscaled_duration < 0 || unscaled_duration > UINT64_MAX / tc_scale)

I think the correct solution is to cast tc_scale at double:

>>if (unscaled_duration < 0 || unscaled_duration > UINT64_MAX / (double)tc_scale)
(Reporter)

Comment 1

2 years ago
Created attachment 8760697 [details]
Bug 1278515 - prevent the truncation of the result from a division to integer.

Review commit: https://reviewboard.mozilla.org/r/58202/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/58202/
Attachment #8760697 - Flags: review?(cpearce)

Updated

2 years ago
Component: Audio/Video → Audio/Video: Playback
Comment on attachment 8760697 [details]
Bug 1278515 - prevent the truncation of the result from a division to integer.

libnestegg is owned by Matthew, so passing review to him.
Attachment #8760697 - Flags: review?(cpearce) → review?(kinetik)
(Assignee)

Comment 3

2 years ago
Comment on attachment 8760697 [details]
Bug 1278515 - prevent the truncation of the result from a division to integer.

https://reviewboard.mozilla.org/r/58202/#review55254

The test condition relies truncation because the conversion of the scaled duration to uint64_t on line 2083 assumes this.  This change would allow unscaled_durations that can overflow to be converted, e.g.:

tc_scale = 1000000
unscaled_duration = 18446744073709.55 (UINT64_MAX / 1000000.0)

(unscaled_duration > UINT64_MAX / tc_scale) == true
(unscaled_duration > UINT64_MAX / (double) tc_scale) == false

So to silence the warning, I think the correct approach is to convert unscaled_duration to uint64_t before the comparison.
Attachment #8760697 - Flags: review?(kinetik) → review-
(Assignee)

Comment 4

2 years ago
I fixed this upstream here: https://github.com/kinetiknz/nestegg/commit/9d35e9eab50fc097379b6517d80a650f5e86f6a8

I'll let the fix make its way into Gecko naturally with the next resync with upstream.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
(Assignee)

Updated

2 years ago
Assignee: bpostelnicu → kinetik
See Also: → bug 1294527
You need to log in before you can comment on or make changes to this bug.