#504 Code issues reported by PVS Studio

[pavel_pimenov]

report:
https://www.fly-server.ru/pvs-studio/owlnext/

Date: 11/13/21 11:43:50
PVS-Studio Version: 7.15.53142.3184
Total Warnings (GA): 1107
Total Warnings (OP): 341
Total Warnings (64): 63

[Vidar Hasfjord]

Hi Pavel,

Thanks for the report!

Many of the reported issues are false positives. For example, the analyser doesn't take into account that some functions (such as TXWindow::Raise) always throw an exception, hence it fails to understand the control flow in many functions, such as in TLayoutWindow::AddConstraint, where it incorrectly reports numerous usages of a null-pointer, despite the impossibility of the code being reached if the pointer is null.

However, having had a brief look at the report, I see there are some good catches that should be fixed, such as missing return statements and incorrect comparisons of BOOL variables.

Feel free to contribute patches for the serious issues!

Edit: I have fixed the following issues on the trunk:

• Comparing BOOL variables against TRUE [r5604]
• Missing return statements [r5605]
• Redundant std::move usages [r5606]
• Typo in TEdit::GetCurrentPosition [r5607]
• Incorrect truncating cast in TListViewCtrl::GetApproxRect [r5608]
• Incorrect cast in THotKey::GetHotKey [r5609]
• Duplicate flag in TGridPicker::GetPopStyle [r5610]
• Superfluous if-statement in TControlBar::TControlBar [r5611]
• Nonsensical PRECONDITION(this) [r5612]
• Unsafe TCelArray assignment operator [r5613]
• Incorrect scanf format in TConfigFile::ReadData [r5614]
• Code smell in local functions in "configfl.cpp" [r5615]
• Unsafe TFindReplaceDialog::TData assignment operator [r5616]
• Unsafe TOpenSaveDialog::TData assignment operator [r5617]
• Unsafe TSocketError assignment operator [r5618]
  
  

6.44 merges: Fixes in [r5609], [r5610], [r5611] and [r5614] were merged into 6.44 in [r5620], [r5621], [r5622] and [r5623], respectively, by Jogy. Further, fixes in [r5604] (partially) and [r5608] were merged into 6.44 in [r5684], by me.

7.0 merges: Fixes in [r5604] (partially), [r5608], [r5609] and [r5614] were merged into 7.0 in [r5683]. Further, fixes in [r5610] and [r5611] were merged into 7.0 in [r5685], bringing 7.0 in line with merged fixes in 6.44.

[Ognyan Chernokozhev]

Good job on making all those fixes. Does it make sense to transfer some of them to the 6.44 and 7.0 branches?

[Vidar Hasfjord]

Thanks, Jogy!

Does it make sense to transfer some of [the fixes] to the 6.44 and 7.0 branches?

Yes, some of the issues warrant fixes to the release branches, provided they don't break ABI compatibility. That said, most of the issues seem unlikely to have any effect. Please review and give your opinion!

There remain many issues in the report yet to be reviewed. Perhaps you (or anyone else) can spot important issues that I have overlooked so far. Note that I have not considered issues in the extension libraries at all. I have only looked at issues in the core.

[Ognyan Chernokozhev]

Regarding [r5608]: Wouldn't it be faster and more self-documenting to make use of TSize::CreateFromPackedExtents instead of creating a temporary TPoint object that is then converted to a TSize?

[Vidar Hasfjord]

Regarding [r5608]: Wouldn't it be faster and more self-documenting to make use of TSize::CreateFromPackedExtents

Yes, good point! (Or good size, I should say!)

I have committed your suggested improvement [r5624].

But note that CreateFromPackedExtents is not available in 6.44 and 7.0. In 7.1 (trunk) it replaces the converting constructor TSize (DWORD packedExtents).

PS. Good work on merging fixes to 6.44. I'll update the version numbers for the 6.44 and 7.0 branches, and prepare for new releases.

[pavel_pimenov]

update report https://www.fly-server.ru/pvs-studio/owlnext/

Date: 12/18/21 18:19:49
PVS-Studio Version: 7.16.55368.3308
Total Warnings (GA): 1064
Total Warnings (OP): 341
Total Warnings (64): 61
Fails/Info: 1

[Vidar Hasfjord]

Fixes for the selected issues listed in my earlier post have now been released in 6.44.16 and 7.0.6.

https://sourceforge.net/p/owlnext/bugs/504/#be75

Since this is only a small subset of the issues reported by PVS Studio, I will leave this ticket open.

Please feel free to review the report and fix remaining issues.