code checking

Post Reply
spacy51
Senior Member
Posts: 371
Joined: Tue Mar 18, 2008 4:59 pm

code checking

Post by spacy51 »

I'm using the VC++ code analysis tool to improve the code quality.

 

I found an unclarity where "u8 attrs" is defined twice in one function.

It would be great if someone who knows the GB code could check that up:

 

gbGfx.cpp

first declaration in line 94

second declaration in line 291

Last edited by spacy51 on Sun Aug 30, 2009 4:23 pm, edited 1 time in total.
spacy51
Senior Member
Posts: 371
Joined: Tue Mar 18, 2008 4:59 pm

code checking

Post by spacy51 »

Here is another issue:

 

gb.cpp, line 1592

Code: Select all

  switch(address & 0xf000) {
 case 0x0a:
 case 0x0b:

 

The case marks can not be reached because of the strange bitmask in the switch statement. Has to be an error. Maybe a bit-shift is missing?

Last edited by spacy51 on Sun Aug 30, 2009 4:02 pm, edited 1 time in total.
spacy51
Senior Member
Posts: 371
Joined: Tue Mar 18, 2008 4:59 pm

code checking

Post by spacy51 »

More around that area:

 

gb.cpp, Line 1637:

case 0x44:
if (((gbHardware & 7) && ((gbLcdMode == 1) && (gbLcdTicks == 0x71))) ||

Code: Select all

      (!(register_LCDC [b]&&[/b] 0x80)))


    return 0;

 

shouldn't that be a bitwise and instead of a logic one?

spacy51
Senior Member
Posts: 371
Joined: Tue Mar 18, 2008 4:59 pm

code checking

Post by spacy51 »

There are several cases of variables like x, or i defined multiple times, but I guess that's negligible.

spacy51
Senior Member
Posts: 371
Joined: Tue Mar 18, 2008 4:59 pm

code checking

Post by spacy51 »

rpi.cpp, line 141:

(pOCur++) = ((pICur & 0xF800) << rshiftDiff) |
((*pICur & 0x07E0) << gshiftDiff) |

Code: Select all

					      ((*pICur & 0x001F) << bshiftDiff);


				[b]*pICur++[/b];

 

 

Can have two meanings: either increment value at address, or increment pointer after accessing value. I don't know what's right.

 

The same issue in line 160.

Last edited by spacy51 on Sun Aug 30, 2009 4:59 pm, edited 1 time in total.
Post Reply