Skip to content

Fix CodeQL warnings#5496

Merged
vadi2 merged 1 commit intodevelopmentfrom
fix-codeql-warnings
Oct 9, 2021
Merged

Fix CodeQL warnings#5496
vadi2 merged 1 commit intodevelopmentfrom
fix-codeql-warnings

Conversation

@vadi2
Copy link
Copy Markdown
Member

@vadi2 vadi2 commented Oct 8, 2021

Brief overview of PR changes/additions

Fix CodeQL warnings - differently-sized integers can lead to an overflow. https://codeql.github.com/codeql-query-help/cpp/cpp-comparison-with-wider-type

Motivation for adding to Mudlet

Fix CodeQL issues that has been sitting around for a year without attention

Screenshot 2021-10-08 8 23 10 AM

Other info (issues closed, discussion etc)

The differently-sized integers aren't really helping all too much, but they do have the potential to cause trouble.

@vadi2 vadi2 requested a review from a team as a code owner October 8, 2021 06:26
@vadi2 vadi2 requested a review from a team October 8, 2021 06:26
@add-deployment-links
Copy link
Copy Markdown

add-deployment-links bot commented Oct 8, 2021

Hey there! Thanks for helping Mudlet improve. 🌟

Test versions

You can directly test the changes here:

No need to install anything - just unzip and run.
Let us know if it works well, and if it doesn't, please give details.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Oct 8, 2021

clang-tidy review says "All clean, LGTM! 👍"

Copy link
Copy Markdown
Member

@SlySven SlySven left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it makes a code-analyser bot happy then who am I to make it ☹️

mpHost->mpConsole->print(prefix, Qt::red, mpHost->mBgColor); // Bright Red
mpHost->mpConsole->print(firstLineTail.append('\n'), QColor(255, 255, 50), mpHost->mBgColor); // Bright Yellow
for (quint8 _i = 0; _i < body.size(); ++_i) {
for (int _i = 0; _i < body.size(); ++_i) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 I guess it is okay for these, I only used a smaller unsigned integer type because the number of line in the message could never be negative and I could not foresee there ever being as many as 256 lines in one of these messages. OTOH Qt insists on using a (standard) signed int for every size() method for each of it's (container) classes...

label_mapFileSaveFormatVersion->setEnabled(false);
if (pHost->mpMap->mMaxVersion > pHost->mpMap->mDefaultVersion || pHost->mpMap->mMinVersion < pHost->mpMap->mDefaultVersion) {
for (short int i = pHost->mpMap->mMinVersion; i <= pHost->mpMap->mMaxVersion; ++i) {
for (int i = pHost->mpMap->mMinVersion; i <= pHost->mpMap->mMaxVersion; ++i) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤷‍♂️ Same again - only this time the integer type miss-match is entirely (my)self-inflicted - this loop populates the map format version number (for saving map files) QComboBox with each of the limited (currently 17, 18, 19 or, the default, 20 - inserted before all the others) values available - I never anticipate the number exceeding 255 so it was semi-appropriate to limit the loop variable to being a short - but then again, I think rain is wet so who am I to say...?

@vadi2 vadi2 merged commit c154e9d into development Oct 9, 2021
@vadi2 vadi2 deleted the fix-codeql-warnings branch October 9, 2021 04:16
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