MSVC Code Analysis warnings - Other

[DanRStevens]

Related: #319

Running MSVC's Code Analysis (Analyze -> Run Code Analysis -> On Solution) gives a number of warnings. Most were suggestions to use enum class (#319). The remainder, not related to enum class are:

Z:\OPHD\src\Map\TileMap.cpp(76): warning C26451: Arithmetic overflow: Using operator '-' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '-' to avoid overflow (io.2).
Z:\OPHD\src\Map\TileMap.cpp(76): warning C26451: Arithmetic overflow: Using operator '-' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '-' to avoid overflow (io.2).
Z:\OPHD\src\Map\TileMap.cpp(77): warning C26451: Arithmetic overflow: Using operator '-' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '-' to avoid overflow (io.2).
Z:\OPHD\src\Map\TileMap.cpp(78): warning C26451: Arithmetic overflow: Using operator '-' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '-' to avoid overflow (io.2).
Z:\OPHD\src\Map\TileMap.cpp(78): warning C26451: Arithmetic overflow: Using operator '+' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '+' to avoid overflow (io.2).
Z:\OPHD\src\Map\TileMap.cpp(80): warning C26451: Arithmetic overflow: Using operator '-' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '-' to avoid overflow (io.2).
Z:\OPHD\src\Map\TileMap.cpp(81): warning C26451: Arithmetic overflow: Using operator '+' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '+' to avoid overflow (io.2).
Z:\OPHD\src\Map\TileMap.cpp(83): warning C26451: Arithmetic overflow: Using operator '+' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '+' to avoid overflow (io.2).
Z:\OPHD\src\Map\TileMap.cpp(83): warning C26451: Arithmetic overflow: Using operator '-' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '-' to avoid overflow (io.2).
Z:\OPHD\src\Map\TileMap.cpp(84): warning C26451: Arithmetic overflow: Using operator '+' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '+' to avoid overflow (io.2).
Z:\OPHD\src\Map\TileMap.cpp(85): warning C26451: Arithmetic overflow: Using operator '+' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '+' to avoid overflow (io.2).
Z:\OPHD\src\Map\TileMap.cpp(85): warning C26451: Arithmetic overflow: Using operator '+' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '+' to avoid overflow (io.2).
Z:\OPHD\src\Map\TileMap.cpp(340): warning C26451: Arithmetic overflow: Using operator '+' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '+' to avoid overflow (io.2).
Z:\OPHD\src\Map\TileMap.cpp(340): warning C26451: Arithmetic overflow: Using operator '+' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '+' to avoid overflow (io.2).
Z:\OPHD\src\UI\Core\ListBox.cpp(244): warning C26451: Arithmetic overflow: Using operator '-' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '-' to avoid overflow (io.2).
Z:\OPHD\src\UI\Core\ListBox.cpp(244): warning C26451: Arithmetic overflow: Using operator '+' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '+' to avoid overflow (io.2).
Z:\OPHD\src\UI\Core\Slider.cpp(27): warning C26495: Variable 'Slider::mSliderType' is uninitialized. Always initialize a member variable (type.6).
Z:\OPHD\src\MicroPather\micropather.cpp(53): warning C26495: Variable 'OpenQueue::sentinelMem' is uninitialized. Always initialize a member variable (type.6).
Z:\OPHD\src\MicroPather\micropather.h(222): warning C26495: Variable 'micropather::PathNode::child' is uninitialized. Always initialize a member variable (type.6).
Z:\OPHD\src\MicroPather\micropather.cpp(298): warning C6011: Dereferencing NULL pointer 'block'. 
Z:\OPHD\src\MicroPather\micropather.cpp(848): warning C26451: Arithmetic overflow: Using operator '+' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '+' to avoid overflow (io.2).
Z:\OPHD\src\MicroPather\micropather.cpp(959): warning C28182: Dereferencing NULL pointer. 'child' contains the same NULL value as 'inOpen' did. See line 937 for an earlier location where this can occur

[DanRStevens]

Hmm, can this issue be closed? I don't see any of these warnings on AppVeyor. Though these warnings might not have appeared there in the first place. Were the Code Analysis results only available locally?

[Brett208]

Dan,

Unsure if you can find these somehow using AppVeyor. Below is the current pull. It looks like many were cleaned up but a few remain.

Warning C26451  Arithmetic overflow: Using operator '+' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '+' to avoid overflow (io.2). OPHD    \OPHD\MAP\TILEMAP.CPP   325 
Warning C26451  Arithmetic overflow: Using operator '+' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '+' to avoid overflow (io.2). OPHD    \OPHD\MAP\TILEMAP.CPP   325 

Warning C26451  Arithmetic overflow: Using operator '-' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '-' to avoid overflow (io.2). OPHD    \OPHD\UI\Core\ListBox.cpp   257 
Warning C26451  Arithmetic overflow: Using operator '+' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '+' to avoid overflow (io.2). OPHD    \OPHD\UI\Core\ListBox.cpp   257 

Warning C26451  Arithmetic overflow: Using operator '+' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '+' to avoid overflow (io.2). OPHD    \OPHD\UI\IconGrid.cpp   163 

** Micropather specific **

Warning C28182  Dereferencing NULL pointer. 'child' contains the same NULL value as 'inOpen' did. See line 937 for an earlier location where this can occur OPHD    \OPHD\MicroPather\micropather.cpp   959 
Warning C26495  Variable 'micropather::PathNode::child' is uninitialized. Always initialize a member variable (type.6). OPHD    \OPHD\MicroPather\micropather.h 222 
Warning C26495  Variable 'micropather::PathNode::inClosed' is uninitialized. Always initialize a member variable (type.6).  OPHD    \OPHD\MicroPather\micropather.h 222 
Warning C26495  Variable 'micropather::PathNode::inOpen' is uninitialized. Always initialize a member variable (type.6).    OPHD    \OPHD\MicroPather\micropather.h 222 

[DanRStevens]

Ahh, ok, that's helpful to know. I'll have to take a peak at some of them.

Would be nice to find some way to generate these warnings on AppVeyor.

[DanRStevens]

I found a way to generate Code Analysis warning on the CI builds, and any warnings will fail the build. Code Analysis can be enabled with the msbuild option:

• /property:RunCodeAnalysis=true.

Currently, the warnings are:

D:\a\OPHD\OPHD\OPHD\MicroPather\micropather.cpp(53): error C26495: Variable 'OpenQueue::sentinelMem' is uninitialized. Always initialize a member variable (type.6). [D:\a\OPHD\OPHD\OPHD\ophd.vcxproj] D:\a\OPHD\OPHD\OPHD\MicroPather\micropather.h(222): error C26495: Variable 'micropather::PathNode::child' is uninitialized. Always initialize a member variable (type.6). [D:\a\OPHD\OPHD\OPHD\ophd.vcxproj] D:\a\OPHD\OPHD\OPHD\MicroPather\micropather.cpp(298): error C6011: Dereferencing NULL pointer 'block'. [D:\a\OPHD\OPHD\OPHD\ophd.vcxproj] D:\a\OPHD\OPHD\OPHD\MicroPather\micropather.cpp(848): error C26451: Arithmetic overflow: Using operator '+' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '+' to avoid overflow (io.2). [D:\a\OPHD\OPHD\OPHD\ophd.vcxproj] D:\a\OPHD\OPHD\OPHD\States\MapViewState.cpp(1014): error C26820: This is a potentially expensive copy operation. Consider using a reference unless a copy is required (p.9). [D:\a\OPHD\OPHD\OPHD\ophd.vcxproj] D:\a\OPHD\OPHD\OPHD\States\MapViewState.cpp(1243): error C26820: This is a potentially expensive copy operation. Consider using a reference unless a copy is required (p.9). [D:\a\OPHD\OPHD\OPHD\ophd.vcxproj] D:\a\OPHD\OPHD\OPHD\States\MapViewStateEvent.cpp(181): error C26820: This is a potentially expensive copy operation. Consider using a reference unless a copy is required (p.9). [D:\a\OPHD\OPHD\OPHD\ophd.vcxproj] D:\a\OPHD\OPHD\OPHD\StructureManager.cpp(47): error C26820: This is a potentially expensive copy operation. Consider using a reference unless a copy is required (p.9). [D:\a\OPHD\OPHD\OPHD\ophd.vcxproj]

[DanRStevens]

I did an updated run with /property:RunCodeAnalysis=true. Looks like the remaining errors are either in Micropather or Google Test (example run logs):

D:\a\OPHD\OPHD\OPHD\MicroPather\micropather.cpp(53): error C26495: Variable 'OpenQueue::sentinelMem' is uninitialized. Always initialize a member variable (type.6). [D:\a\OPHD\OPHD\OPHD\ophd.vcxproj] D:\a\OPHD\OPHD\OPHD\MicroPather\micropather.h(222): error C26495: Variable 'micropather::PathNode::child' is uninitialized. Always initialize a member variable (type.6). [D:\a\OPHD\OPHD\OPHD\ophd.vcxproj] D:\a\OPHD\OPHD\OPHD\MicroPather\micropather.cpp(298): error C6011: Dereferencing NULL pointer 'block'. [D:\a\OPHD\OPHD\OPHD\ophd.vcxproj] D:\a\OPHD\OPHD\OPHD\MicroPather\micropather.cpp(848): error C26451: Arithmetic overflow: Using operator '+' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '+' to avoid overflow (io.2). [D:\a\OPHD\OPHD\OPHD\ophd.vcxproj]

D:\a\OPHD\OPHD\vcpkg_installed\x86-windows\x86-windows\include\gtest\internal\gtest-port.h(1380): error C26495: Variable 'testing::internal::Mutex::criticalsection' is uninitialized. Always initialize a member variable (type.6). [D:\a\OPHD\OPHD\testLibOPHD\testLibOPHD.vcxproj] D:\a\OPHD\OPHD\vcpkg_installed\x86-windows\x86-windows\include\gtest\gtest-matchers.h(335): error C26439: This kind of function should not throw. Declare it 'noexcept' (f.6). [D:\a\OPHD\OPHD\testLibOPHD\testLibOPHD.vcxproj]

D:\a\OPHD\OPHD\vcpkg_installed\x86-windows\x86-windows\include\gtest\internal\gtest-port.h(1380): error C26495: Variable 'testing::internal::Mutex::criticalsection' is uninitialized. Always initialize a member variable (type.6). [D:\a\OPHD\OPHD\testLibControls\testLibControls.vcxproj] D:\a\OPHD\OPHD\vcpkg_installed\x86-windows\x86-windows\include\gtest\gtest-matchers.h(335): error C26439: This kind of function should not throw. Declare it 'noexcept' (f.6). [D:\a\OPHD\OPHD\testLibControls\testLibControls.vcxproj]

That means we have no Code Analysis warnings generated by our own code. Of course this means we still can't enable /property:RunCodeAnalysis=true by default on CI builds, since they would fail.

The Google Test ones from the new test projects are curious though. I would expect anything Google Test related to be treated as external code, and so excluded from Code Analysis.