Closed Bug 1278515 Opened 4 years ago Closed 4 years ago

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

Categories

(Core :: Audio/Video: Playback, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Tracking Status
firefox50 --- affected

People

(Reporter: andi, Assigned: kinetik)

References

(Blocks 1 open bug)

Details

(Keywords: coverity, Whiteboard: CID 1362113)

Attachments

(1 file)

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)
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)
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-
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
Closed: 4 years ago
Resolution: --- → FIXED
Assignee: bpostelnicu → kinetik
See Also: → 1294527
You need to log in before you can comment on or make changes to this bug.