Author Topic: SurfaceSet::loadPck()  (Read 6385 times)

Offline sir_nacnud

  • Captain
  • ***
  • Posts: 54
    • View Profile
SurfaceSet::loadPck()
« on: February 03, 2011, 07:01:12 am »
I figured out the bug that was causing setPixelIterative() to walk past the end of the buffer.  I submitted a patch earlier that prevents setPixel() from going past the end of the buffer, but this fixes the actual problem.

Here's what was going on, keep in mind this is only apparent on Android.

In SurfaceSet::loadPck(), after the first read happens and we skip x amount of pixels via two nested for loops, then we go in to a while loop.  This while loop continues until we hit the end of the spite, represented by 0xFF in the file.  The problem is on Android we never hit the if or else if branch and the while loop runs to the end of the file.  Then we start the next iteration of the main for loop at the end of the file.  When we do that first read, we get back 0xFF, and attempt to skip more pixels than we have.  This is where the crash in setPixelIterative()/setPixel() happens.  The reason we never hit the if or else if branches on Android is because char is unsigned, so 0xFF and 0xFE get get converted to 255 and 254 when compared to -1 and -2.  On the desktop, char is signed, so 0xFF and 0xFE get correctly converted to -1 and -2.  My patch casts both -1 and -2 as chars before comparing them to the value we read.  This fixes works both on Android and the desktop.

Offline SupSuper

  • Lazy Developer
  • Administrator
  • Commander
  • *****
  • Posts: 2162
    • View Profile
Re: SurfaceSet::loadPck()
« Reply #1 on: February 03, 2011, 11:37:57 pm »
Thanks for the heads-up. The bytes should always be interpreted as unsigned 99% of the time anyways, so I just changed it to unsigned throughout to be consistent.