[Bf-committers] Re: [Bf-blender-cvs] CVS commit: blender/source/blender/blenloader/intern writefile.c

[Alexander Ewering]

On Fri, 27 Oct 2006, Kent Mein wrote:

> sirdude (Kent Mein) 2006/10/27 20:24:10 CEST
>
>  Modified files:
>    blender/source/blender/blenloader/intern writefile.c
>
>  Log:
>  more bugs found thanks to klockwork
>  all of these are just check a var to make sure it points to something before
>  using them.

I know I'm not very involved with Blender development for a long time, but
basically, all that you are adding is checking the return value of malloc in
each and every place.

This discussion has occured in the past on the mailing list, and it was
agreed on (IIRC) that if malloc fails to allocate a mere 20 bytes for a
filename, then your system's state is so hosed that you have more to worry
about than a segfaulting application. And as Blender uses an malloc wrapper
anyway, it would be far wiser to check this in a central place, call an
"Emergency save" function, and exit.

if ((wd == NULL) || wd->error || (mem == NULL) || memlen < 1) return;
if (wd->error) return;

Apart from being redundant, why would anyone ever pass NULL to a function
that is supposed to do something with the argument? It would be the task of
the calling function to make sure that it doesn't pass NULL.

This slightly thoughtless "do everything so that code analysis tool foo / bar 
is quiet" probably more increases noise in the code than what it's worth.

Just a hint in the dark... ;)

| alexander ewering              instinctive mediaworks
| ae[@]instinctive[.]de   http://www[.]instinctive[.]de