Skip to content

Work around CodeQL multiplication warning#2456

Closed
softins wants to merge 1 commit intojamulussoftware:masterfrom
softins:size-warnings
Closed

Work around CodeQL multiplication warning#2456
softins wants to merge 1 commit intojamulussoftware:masterfrom
softins:size-warnings

Conversation

@softins
Copy link
Copy Markdown
Member

@softins softins commented Mar 3, 2022

Short description of changes

Work around CodeQL multiplication overflow warning by performing the multiply in advance.
This is also slightly more efficient, as the product is potentially used twice.

CHANGELOG: SKIP

Context: Fixes an issue?

Fixes a CodeQL warning in the CI. No user-visible difference.

Does this change need documentation? What needs to be documented and how?

No

Status of this Pull Request

Complete. Didn't find any more potential instances.

What is missing until this pull request can be merged?

CI success for Android build

Checklist

  • I've verified that this Pull Request follows the general code principles
  • I tested my code and it does what I want
  • My code follows the style guide
  • I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
  • I've filled all the content above

@pljones
Copy link
Copy Markdown
Collaborator

pljones commented Mar 3, 2022

CHANGELOG: SKIP

It fixes a CodeQL issue, so it's a definite improvement (i.e. builds won't fail annoyingly). So I think it should have a change log entry.

@softins
Copy link
Copy Markdown
Member Author

softins commented Mar 3, 2022

Actually, I've just noticed a more general issue in android/sound.cpp which I think was an oversight missed out of #2237, so I'll update this PR to cover both, as "improvements to Android sound driver".

@softins
Copy link
Copy Markdown
Member Author

softins commented Mar 3, 2022

In fact, I'll just close this and raise a fresh PR with more meaningful branch name.

@softins softins closed this Mar 3, 2022
@softins softins deleted the size-warnings branch August 31, 2023 10:23
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.

2 participants