[Scummvm-devel] coverity

Johannes Schickel lordhoto at gmail.com
Sat Nov 9 23:19:41 CET 2013


On 11/09/2013 08:41 PM, Eugene Sandulenko wrote:
> On 9 November 2013 15:49, Johannes Schickel <lordhoto at gmail.com 
> <mailto:lordhoto at gmail.com>> wrote:
>
>     On 10/30/2013 12:24 PM, Eugene Sandulenko wrote:
>     > On 30 October 2013 12:52, Willem Jan Palenstijn <wjp at usecode.org
>     <mailto:wjp at usecode.org>
>     > <mailto:wjp at usecode.org <mailto:wjp at usecode.org>>>wrote:
>     >
>     >     In particular, blindly initializing variables in constructors
>     >     while they
>     >     _should_ already be initialized in some init() method just hides
>     >     potential bugs
>     >     in init() or missed calls to init(). On top of that, it makes
>     >     tools like
>     >     valgrind or better static analysis useless for finding and
>     >     catching any such bugs.
>     >
>     >
>     > I agree, however I am the primarily reason for this email being
>     sent.
>     >
>     >
>
>     I am bit confused about the recent commits 6491ad1cf9...38d02d1687
>     (among others, but that's the biggest batch I've seen ;-)). I thought
>     there was an agreement to Willem Jan's post stating that these
>     initialization changes were not the best idea and thus we shouldn't do
>     them. But maybe I didn't get this right?
>
>
> There are several types of unitialized variables. One big chunk is 
> basically a false positive since we use Engine::init() methods, and 
> then there are tons of really uninitalized stuff. I am really glad 
> that coverity gives me possibility to fix those obscure bugs and point 
> out variables which are initialized with garbage on MacOS and Linux, 
> but work fine on Windows.
>

I think nobody questioned that initializing variables which actually are 
used uninitialized is the correct thing to do (nor that using static or 
dynamic analysis tools to find such cases is a good thing).

> I agree with Willem that it is not really good to address first part 
> in many cases, but not in all. If you take a look, you will see, that 
> in these cases the variables are initialized much deeper that 
> ::init(), and some further refactoring could potentially lead to 
> unstable behavior. So in my opinion in these cases it is cheaper and 
> better to plug the hole altogether.
>

Isn't the unstable behavior you talk about a clearly visible indicator 
for an actual regression from the refactoring whereas with the simple 
variable initialization it might be hidden (but not fixed) and thus make 
it harder to figure out the problem?

// Johannes




More information about the Scummvm-devel mailing list