Article about bugs found in VCMI

International board Help & Bugs
#1

Dear VCMI core contributors,

Thank you very much for developing the VCMI project. I am a big fan of the Heroes of Might and Magic series.

Having seen your project on github, I decided to check it with our static analyzer and wrote an article based on the results of the check. Here is a link to the article: Heroes of Code and Magic: VCMI game engine analysis. You might be interested in taking a look.

Yours sincerely,
Alexey Smolskas,
C++ Analyzer Developer,
PVS-Studio LLC

1 Like
#2

Thanks!
Will probably go through them a bit later, but don’t see anything critically important. A lot of them are about cases that should never happen to begin with. For example Fragment N1 can never be called with such parameter.

Fragment N14
Looks to be false positive - owner.executeStagedAnimations(); can call functors, which in the end will add elements to currentAnimations container. A bit tricky code, but we want animaitons to run seamlessly, without pause.

Fragment N10
Fragment N22
Already fixed I believe

1 Like
#3

“Failed allocations” are delusional to begin with and can only be handled on embedded devices which have very limited memory, or some farms which try to manage tons of memory between hundreds of processes.

In N3, if we failed to alocate memory for modificator, Random Map Generator would get stuck forever anyway. Then, random map actually needs a lot of free memory remaining at this point. You can’t do much if you have 0 free bytes remaining…

#4

Yeah, a lot of those are basically unreachable code.

Or attempt to handle cases that can’t be handled anyway. Like sure, we can add check for dynamic_cast, but if we reached this scenario then this means that game state got seriously broken before and will do something stupid soon anyway so “clean” crash on nullptr dereference might be better than obscure crash down the line.