Author Topic: Surface::setPixel() bug  (Read 6608 times)

Offline sir_nacnud

  • Captain
  • ***
  • Posts: 54
    • View Profile
Surface::setPixel() bug
« on: January 22, 2011, 06:39:52 pm »
I found a bug in Surface::setPixel(), the checks to see if x and y are valid allow x to be equal to width and y equal to height.  If x or y are width or height, the pixel set will be wrong, and if both are width and height, you will go past the end of the pixel buffer.  See attached patch for the fix.

We actually do go past the end of the buffer when loading spks files.  The culprit is Surface::setPixelIterative() in conjunction with Surface::loadSpk().  I am not for sure why people haven't been seeing a crash, I guess we got lucky with the memory past the pixel buffer.  I will be looking in to why we are trying to write more pixel data than we allocated.  I will also probably be rewriting the loadSpk() and loadScr(), as using setPixelIterative() and setPixel() aren't efficient ways to load the pixel data.

Offline SupSuper

  • Lazy Developer
  • Administrator
  • Commander
  • *****
  • Posts: 2162
    • View Profile
Re: Surface::setPixel() bug
« Reply #1 on: January 25, 2011, 08:07:36 am »
Thanks, comitted.

I can't find any reason why loadSpk would be breaking buffers though.

Offline sir_nacnud

  • Captain
  • ***
  • Posts: 54
    • View Profile
Re: Surface::setPixel() bug
« Reply #2 on: January 26, 2011, 03:38:27 am »
It was for some reason on GRAPHS.SPK.  I will look in to it to see what the problem was.

Offline sir_nacnud

  • Captain
  • ***
  • Posts: 54
    • View Profile
Re: Surface::setPixel() bug
« Reply #3 on: February 03, 2011, 07:02:31 am »
Turns out SurfaceSet::loadPck() was the problem: https://openxcom.ninex.info/forum/index.php?topic=169.0