Talk:LoadBMPCpp

From GPWiki

I can see a few problems with this file.

Firstly it doesn't take into account bitmaps which are not a width which is a mutiple of 4, ie an image with a size such as 31*32. images like these have padding which is just beening read in as if it was part of the data and it therefore misses some of the data out and will make the data corrupt.

Secondly not all bitmaps need to be flipped, only bitmaps with a height greater than zero have there y order reversed.

Thirdly the line if(pData[0x0]!='B' || pData[0x1]!='M') // BMP ID Bytes, should be 'BM' return IMG_ERR_BAD_FORMAT; does not guarantee that the type is "BM", why not check the magic number for bitmaps is correct (0x4D42 or 19778)

Security/Portability Issues

I see problems too.

- Portability

 This assumes the CPU is in little endian order. Byte-swapping
 should be performed.

- Security

 No checks for integer overflow done.

- Various nitpick

 const incorrect-ness (use const char *szFilename,
 not char *szFilename )

 use RAII-capable standard containers like std::vector
 instead of manual heap management.

Thanks for your input guys.

I do intend to update the code to address these issues. However, given that it works for 99.9% of images I throw at it in its current form, it's hard to prioritise it over other stuff I'm working on. Codehead 09:24, 24 January 2007 (EST)