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

Status: RESOLVED FIXED

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

Description

[Andi [:andi]]

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)

Comment 1

[Andi [:andi]]

Attachment: 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/

Comment 2

[Chris Pearce [:cpearce (Not reading bugmail)]]

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.

Comment 3

[Matthew Gregan [: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.

Comment 4

[Matthew Gregan [:kinetik]]

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.