Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Guard against division by zero #404

Merged
merged 2 commits into from
Nov 15, 2022

Conversation

tstarling
Copy link
Contributor

  • Add getid3_lib::SafeDiv() which does a division with ternary guard.
    Avoids repeating the denominator expression.
  • Rearrange a few formulas to avoid unnecessary divisions.
    a/(b/c) = a*c/b except that the latter produces an answer when c=0.
  • Remediate divisions that may possibly divide by zero, or look
    suspicious.
  • In module.audio.midi.php: remove unused variable
  • In module.audio.mp3.php: raise an error for zero channels, since lots
    of things divide by the channel count.
  • In module.audio.voc.php: ignore channels=0, pretend channels=1
  • In module.graphic.bmp.php: with ExtractData the RLE modes will divide
    by zero if width=0. With any compression mode, zero width or height
    means empty data, so add that as a shortcut.

Motivation: https://phabricator.wikimedia.org/T289189

* Add getid3_lib::SafeDiv() which does a division with ternary guard.
  Avoids repeating the denominator expression.
* Rearrange a few formulas to avoid unnecessary divisions.
  a/(b/c) = a*c/b except that the latter produces an answer when c=0.
* Remediate divisions that may possibly divide by zero, or look
  suspicious.
* In module.audio.midi.php: remove unused variable
* In module.audio.mp3.php: raise an error for zero channels, since lots
  of things divide by the channel count.
* In module.audio.voc.php: ignore channels=0, pretend channels=1
* In module.graphic.bmp.php: with ExtractData the RLE modes will divide
  by zero if width=0. With any compression mode, zero width or height
  means empty data, so add that as a shortcut.
@JamesHeinrich
Copy link
Owner

Thanks for this contribution.

One thing I noticed, if you can fix before I merge it:

getid3/module.audio-video.ivf.php:48
it should be timebase_numerator/timebase_denominator but you have denominator twice

@tstarling
Copy link
Contributor Author

getid3/module.audio-video.ivf.php:48 it should be timebase_numerator/timebase_denominator but you have denominator twice

Done. Sorry I missed this. I added SafeDiv after I was halfway through, so in this line I first added a ternary first and then incorrectly changed it to SafeDiv.

@JamesHeinrich JamesHeinrich merged commit ce38a25 into JamesHeinrich:master Nov 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants