Opened 9 years ago

Closed 8 years ago

Last modified 8 years ago

#5458 closed defect (fixed)

SWORD25: Code analysis warnings

Reported by: Templier Owned by:
Priority: normal Component: Engine: Sword25
Keywords: build Cc:
Game: Broken Sword 2.5

Description

Here are the warnings I get when compiling the engine 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.

Note: I have silened most of the warnings in lua & art for now (most are NULL dereference warnings for malloc)

d:\sources\scummvm\scummvm\engines\sword25\util\lua\ldo.cpp(118): warning C4611: interaction between '_setjmp' and C++ object destruction is non-portable
This is not an analysis warning. It might need to be disabled in the property file when generating the project files with create_msvc if deemed invalid

d:\sources\scummvm\scummvm\engines\sword25\kernel\log.cpp(124): warning C6053: Call to '_snprintf' might not zero-terminate string 'ExtFormat'
d:\sources\scummvm\scummvm\engines\sword25\package\packagemanager.cpp(189): warning C6283: 'buffer' is allocated with array new [], but deleted with scalar delete
d:\sources\scummvm\scummvm\engines\sword25\gfx\graphicengine.cpp(229): warning C6011: Dereferencing NULL pointer 'fillRectPtr'
d:\sources\scummvm\scummvm\engines\sword25\fmv\theora_decoder.cpp(248): warning C6011: Dereferencing NULL pointer 'value'

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: #3090234. Ticket imported from: bugs/5458.

Change History (5)

comment:1 by fingolfin, 9 years ago

Component: Engine: Sword25
Game: Broken Sword 2.5

comment:2 by sev-, 9 years ago

Can you please rerun it for the current code?

comment:3 by Templier, 9 years ago

Here are the filtered results (most of them being malloc/realloc related):

d:\sources\scummvm\scummvm-jt\engines\sword25\gfx\image\art.cpp(106): warning C6011: Dereferencing NULL pointer '*p_vpath': Lines: 101, 103, 104, 105, 106
d:\sources\scummvm\scummvm-jt\engines\sword25\gfx\image\art.cpp(105): warning C6308: 'realloc' might return null pointer: assigning null pointer to '*p_vpath', which is passed as an argument to 'realloc', will cause the original memory block to be leaked
d:\sources\scummvm\scummvm-jt\engines\sword25\gfx\image\art.cpp(106): warning C6011: Dereferencing NULL pointer '*p_vpath': Lines: 101, 103, 104, 105, 106
d:\sources\scummvm\scummvm-jt\engines\sword25\gfx\image\art.cpp(173): warning C6308: 'realloc' might return null pointer: assigning null pointer to 'svp', which is passed as an argument to 'realloc', will cause the original memory block to be leaked
d:\sources\scummvm\scummvm-jt\engines\sword25\gfx\image\art.cpp(210): warning C6308: 'realloc' might return null pointer: assigning null pointer to 'svp', which is passed as an argument to 'realloc', will cause the original memory block to be leaked
d:\sources\scummvm\scummvm-jt\engines\sword25\gfx\image\art.cpp(236): warning C6308: 'realloc' might return null pointer: assigning null pointer to 'points', which is passed as an argument to 'realloc', will cause the original memory block to be leaked
d:\sources\scummvm\scummvm-jt\engines\sword25\gfx\image\art.cpp(252): warning C6308: 'realloc' might return null pointer: assigning null pointer to 'svp', which is passed as an argument to 'realloc', will cause the original memory block to be leaked
d:\sources\scummvm\scummvm-jt\engines\sword25\gfx\image\art.cpp(270): warning C6011: Dereferencing NULL pointer 'svp': Lines: 143, 144, 145, 146, 147, 148, 149, 150, 151, 153, 154, 155, 158, 159, 160, 161, 162, 164, 166, 168, 248, 270
d:\sources\scummvm\scummvm-jt\engines\sword25\gfx\image\art.cpp(465): warning C6308: 'realloc' might return null pointer: assigning null pointer to 'vec', which is passed as an argument to 'realloc', will cause the original memory block to be leaked
d:\sources\scummvm\scummvm-jt\engines\sword25\gfx\image\art.cpp(472): warning C6011: Dereferencing NULL pointer 'vec': Lines: 445, 446, 447, 448, 450, 451, 452, 453, 458, 459, 461, 464, 466, 467, 468, 469, 470, 471, 472
d:\sources\scummvm\scummvm-jt\engines\sword25\gfx\image\art.cpp(1071): warning C6308: 'realloc' might return null pointer: assigning null pointer to 'pq->items', which is passed as an argument to 'realloc', will cause the original memory block to be leaked
d:\sources\scummvm\scummvm-jt\engines\sword25\gfx\image\art.cpp(1165): warning C6308: 'realloc' might return null pointer: assigning null pointer to 'swr->n_points_max', which is passed as an argument to 'realloc', will cause the original memory block to be leaked
d:\sources\scummvm\scummvm-jt\engines\sword25\gfx\image\art.cpp(1171): warning C6011: Dereferencing NULL pointer 'swr->n_points_max': Lines: 1124, 1125, 1126, 1127, 1128, 1129, 1130, 1132, 1133, 1134, 1135, 1152, 1157, 1158, 1159, 1160, 1161, 1164, 1165, 1168, 1169, 1170, 1171
d:\sources\scummvm\scummvm-jt\engines\sword25\gfx\image\art.cpp(1168): warning C6011: Dereferencing NULL pointer 'svp': Lines: 1124, 1125, 1126, 1127, 1128, 1129, 1130, 1132, 1133, 1134, 1135, 1152, 1157, 1158, 1159, 1160, 1161, 1164, 1165, 1168
d:\sources\scummvm\scummvm-jt\engines\sword25\gfx\image\art.cpp(1196): warning C6308: 'realloc' might return null pointer: assigning null pointer to 'seg->points', which is passed as an argument to 'realloc', will cause the original memory block to be leaked
d:\sources\scummvm\scummvm-jt\engines\sword25\gfx\image\art.cpp(1197): warning C6011: Dereferencing NULL pointer 'seg->points': Lines: 1185, 1186, 1187, 1189, 1193, 1194, 1195, 1196, 1197
d:\sources\scummvm\scummvm-jt\engines\sword25\gfx\image\art.cpp(1232): warning C6011: Dereferencing NULL pointer 'result->svp': Lines: 1221, 1222, 1224, 1225, 1226, 1228, 1229, 1230, 1232
d:\sources\scummvm\scummvm-jt\engines\sword25\gfx\image\art.cpp(1393): warning C6308: 'realloc' might return null pointer: assigning null pointer to 'seg->stack', which is passed as an argument to 'realloc', will cause the original memory block to be leaked
d:\sources\scummvm\scummvm-jt\engines\sword25\gfx\image\art.cpp(1394): warning C6011: Dereferencing NULL pointer 'seg->stack': Lines: 1389, 1390, 1392, 1393, 1394
d:\sources\scummvm\scummvm-jt\engines\sword25\package\packagemanager.h(147): warning C6387: 'argument 1' might be '0': this does not adhere to the specification for the function 'strcpy': Lines: 143, 144, 145, 146, 147
d:\sources\scummvm\scummvm-jt\engines\sword25\package\packagemanager.h(149): warning C6011: Dereferencing NULL pointer 'result': Lines: 143, 144, 145, 146, 147, 148, 149

There is also a lot of warnings related to the art_new macro (unchecked allocation using malloc).

And a couple in the lua code:
d:\sources\scummvm\scummvm-jt\engines\sword25\util\lua\lvm.cpp(673): warning C6011: Dereferencing NULL pointer 'pstep': Lines: 374, 375, 376, 377, 378, 379, 380, 381, 382, 383, 386, 387, 388, 398, 399, 400, 401, 402, 403, 404, 386, 387, 388, 398, 399, 400, 401, 402, 403, 404, 386, 387, 388, 398, 399, 400, 401, 402, 662, 663, 664, 665, 666, 667, 669, 671, 672, 673
d:\sources\scummvm\scummvm-jt\engines\sword25\util\lua\lvm.cpp(673): warning C6011: Dereferencing NULL pointer 'pstep': Lines: 374, 375, 376, 377, 378, 379, 380, 381, 382, 383, 386, 387, 388, 398, 399, 400, 401, 402, 403, 404, 386, 387, 388, 398, 399, 400, 401, 402, 403, 404, 386, 387, 388, 398, 399, 400, 401, 402, 662, 663, 664, 665, 666, 667, 669, 671, 672, 673

comment:4 by Templier, 8 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 Templier, 8 years ago

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.