#5451 closed defect (fixed)
COMMON/BASE/BACKENDS: Code analysis warnings
| Reported by: | Templier | Owned by: | |
|---|---|---|---|
| Priority: | normal | Component: | Ports |
| Version: | Keywords: | build | |
| Cc: | Game: |
Description
Here are the warnings I get when compiling using Code Analysis in VS2010. I tried to remove all the obvious false positives. Some warnings look suspicious, others are pointing to correct code that still might benefit from better error checking.
COMMON: d:\sources\scummvm\scummvm\common\macresman.cpp(671): warning C6011: Dereferencing NULL pointer 'iconData' d:\sources\scummvm\scummvm\common\algorithm.h(245): warning C4146: unary minus operator applied to unsigned type, result still unsigned d:\sources\scummvm\scummvm\sound\timestamp.cpp(35) : see reference to function template instantiation 'T Common::gcd<uint>(T,T)' being compiled with [ T=uint ] d:\sources\scummvm\scummvm\common\algorithm.h(246): warning C4146: unary minus operator applied to unsigned type, result still unsigned (tthe last two are not code-analysis related and have been there for quite a long time)
BACKENDS: d:\sources\scummvm\scummvm\backends\platform\sdl\sdl.cpp(384): warning C6031: Return value ignored: 'GetWindowsDirectoryA' d:\sources\scummvm\scummvm\backends\platform\sdl\sdl.cpp(396): warning C6031: Return value ignored: 'GetWindowsDirectoryA' d:\sources\scummvm\scummvm\backends\platform\sdl\sdl.cpp(527): warning C6031: Return value ignored: 'sscanf' d:\sources\scummvm\scummvm\backends\platform\sdl\sdl.cpp(542): warning C6031: Return value ignored: 'sscanf' d:\sources\scummvm\scummvm\backends\platform\sdl\sdl.cpp(548): warning C6031: Return value ignored: 'sscanf' d:\sources\scummvm\scummvm\backends\platform\sdl\sdl.cpp(543): warning C6054: String 'color' might not be zero-terminated
BASE: d:\sources\scummvm\scummvm\base\commandline.cpp(305): warning C6011: Dereferencing NULL pointer 'argv'
Note: If you want to try it yourself, you will need: - the create_project patch set, along with the WIP code analysis patch (http://bitbucket.org/Littleboy/scummvm-jt/src) - the following patch to silence some false positives ( http://bitbucket.org/Littleboy/scummvm-jt/src/tip/COMMON%20-%20Add%20annotations%20for%20analysis%20build%20configuration.patch ) - Visual Studio 2010 Ultimate or Team System (create_project support for analysis with VS2005/2008 isn't complete yet)
Ticket imported from: #3087922. Ticket imported from: bugs/5451.
Change History (6)
comment:1 by , 13 years ago
comment:2 by , 13 years ago
Here are the results (I've silenced a couple of false positives and added asserts after malloc calls as needed in my analysis branch):
d:\sources\scummvm\scummvm-jt\base\commandline.cpp(518): warning C6387: 'argument 1' might be '0': this does not adhere to the specification for the function 'strtol': Lines: 305, 308, 316, 317, 319, 320, 322, 333, 335, 338, 341, 344, 349, 355, 359, 366, 369, 372, 375, 378, 381, 384, 387, 390, 393, 396, 399, 408, 413, 416, 419, 422, 425, 428, 431, 436, 442, 451, 454, 457, 460, 463, 466, 472, 481, 490, 493, 496, 499, 508, 511, 515, 518 d:\sources\scummvm\scummvm-jt\backends\platform\sdl\win32\win32.cpp(118): warning C6031: Return value ignored: 'GetWindowsDirectoryA' d:\sources\scummvm\scummvm-jt\backends\platform\sdl\win32\win32.cpp(130): warning C6031: Return value ignored: 'GetWindowsDirectoryA' d:\sources\scummvm\scummvm-jt\backends\platform\sdl\sdl.cpp(403): warning C6054: String 'color' might not be zero-terminated: Lines: 377, 378, 379, 381, 386, 390, 391, 396, 397, 398, 399, 400, 403
comment:3 by , 13 years ago
The strtol warning is a false positive: If the first arg "option" is 0, then usage() is called, which never returns as it calls exit.
Can't say much about the GetWindowsDirectory stuff; but looking at MSDN, it should be fairly easy to properly handle its return values. I'll take a peek eventually, but wouldn't mind if somebody beat me to it ;). Oh yeah, GetWindowsDirectory could in theory overflow the output buffer, in which case we could just call it again on a bigger buffer. An elegant solution here might be to write a simple wrapper for GetWindowsDirectory which returns a Common::String and internally does the "right" thing. It would also have to handle the (rare?) case that GetWindowsDirectory returns a genuine error, though.
The error in sdl.cpp (about the code in OSystem_SDL::setupIcon()) is stricly speaking correct; but then we are talking about a parsing a built-in XPM icon file. Nothing that can fail here... The real bug is that we use sscanf etc. to parse an XPM icon instead of simply putting the palette of the icon in one array, and the image data into a byte array, and then using that directly. Kind of a waste :).
comment:4 by , 13 years ago
Most of the warnings are now corrected/silenced (with master + analysis branch). Will re-run during 1.4 testing.
Current list (with analysis branch): https://gist.github.com/1008034
comment:5 by , 13 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
comment:6 by , 5 years ago
| Component: | → Ports |
|---|
I fixed part of the warnings. Could you please rerun it?