Random "bugs" found with static checker

[GoogleCodeExporter]

I noticed that some work is done to compile mupen64plus with -Wall and to
find bugs with it. So I tried just for fun to compile it with ccc-
interesting methods to find possible errors in c projects. Maybe you are
interested in the things it found.
Another interesting tool is cgcc from sparse. Ok, it will only help to find
minor stuff - it is definitly more helpful for kernel development because
it can detect errors in lock handling.

Small information about some warnings:
* "non-ANSI function declaration of function " - ansi function declarations
have to be like "int test(void)" and not "int test()".
* "symbol 'disasm' was not declared. Should it be static?" - c function
symbols have no type informations in them like C++ function symbols. So
function definitions should be after their declaration (so include the
header files before usage) because the compiler can check if both are the
same. static functions don't generate functions symbols and so they wont
have that problem - the compiler can also optimize their code out if they
aren't used outside of the object file and they are inlined somewhere

The rest should be relative clear

Original issue reported on code.google.com by sven@narfation.org on 28 Sep 2008 at 3:58

Attachments:

• scan-build-2008-09-28-11.tar.bz2
• output.txt
• cgcc.txt

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

Misha, that you? I know static code analysis is a good idea. :)

First off, I assume you're using the latest svn? (r1085)

I've been using gcc 4.3 on Debian. Right now there's no warnings (maybe 2  
uncommitted things in the RSP plugin - I forget) on my setup even with -Wall. I 
think I might need to run through with gcc 4.1 or older if people on these 
setups 
are getting lots of warnings.

It doesn't seem like a lot of there warnings are all that major. Just for your 
knowledge, anything with a 7zip can be more or less dismissed, as that code is 
going to be replaced.

80% of errors you reported are non-ANSI C errors. I'm not sure if we're even 
aiming 
for ANSI strict (note all the C++ style comments), though nice to know about 
the 
(void) issue. The struct errors also fall under this, as ANSI C needs a typedef 
struct the way they're being used (I believe). Another big one is lacking 
const, 
char* with our homegrown translation tr() function. tr() is also on the 
chopping 
block as we eventually want to use GNU gettext. I even have some uncommitted 
work 
in that direction, but its not finished.

I'm a little confused about the "symbol 'disasm' was not declared. Should it be 
static?" type errors. -Wall takes care of a number of those (implict function 
warnings). We did have a bunch of warnings in the recompiler since a header 
wasn't 
being included, but it was fixed recently. My guess is that if they're not 
declared 
in the header, they probably can be static, but it will take a bunch of testing 
to 
confirm that, especially if the plugins hook core functions they shouldn't be. 
I'm 
not sure how much of an optimization this will generate though.

Anyway... good work. 

Original comment by sknau...@wesleyan.edu on 30 Sep 2008 at 5:47

[GoogleCodeExporter]

Who is Misha?

It was r1084. Yes, the warnings of sparse are mostly non-ANSI stuff (keep an 
eye on
the !x & y - this could be a really error). The most important stuff are 
reported by
scan-build/clang (llvm).

And if you really think that static checking is a good thing - try to get in 
contact
with opensource@klocwork.com to get the code checked by some commercial tools.
PS: gcc reports only implicit declarations when you use a function somewhere - 
not
when you define it without declaration.

Original comment by sven@narfation.org on 6 Oct 2008 at 12:00

[GoogleCodeExporter]

Misha was a guy I met a few weeks ago. He did his masters in static code 
evaluation 
and was interested in Mupen64Plus. Great minds think alike, so sorry if you're 
actually someone else.

As far as more professional scanning. Its sort of sad, but we know parts of 
Mupen64Plus are not written very well. 7zip stuff, Rice, Blight, etc. a lot of 
that 
bad code is getting rewritten by capable developers, but this takes time. Once 
we 
fix the known errors, we can go looking for more. 

Original comment by sknau...@wesleyan.edu on 11 Oct 2008 at 6:20

[GoogleCodeExporter]

Thanks for running this code analysis.  I fixed all of the warnings that were 
worth
fixing (almost everything except the 7zip issues) in commit 1121.  The 'ccc' 
warnings
were worthless (it couldn't parse the struct definition properly and complained 
about
non-const format strings).  Most of the cgcc warnings were pedantic but there 
were a
few worthwhile things in there.

Original comment by richard...@gmail.com on 15 Oct 2008 at 5:06

• Changed state: Fixed