Ryan, can you double check that this isn't active in SDL 2.0 or sdl12-compat?

(Sorry if you get several emails like this, we're marking a bunch of bugs.)

We're hoping to ship SDL 2.0.11 on a much shorter timeframe than we have historically done releases, so I'm starting to tag bugs we hope to have closed in this release cycle.

Note that this tag means we just intend to scrutinize this bug for the 2.0.11 release: we may fix it, reject it, or even push it back to a later release for now, but this helps give us both a goal and a wishlist for the next release.

If this bug has been quiet for a few months and you have new information (such as, "this is definitely still broken" or "this got fixed at some point"), please feel free to retest and/or add more notes to the bug.

--ryan.

I can't download the attached file ... browser blocks it ?! can you compress/archive it or something ?

@Sylvain:  try doing:
 wget -O icon.bmp "https://bugzilla.libsdl.org/attachment.cgi?id=3689"


As for the issue: This bmp reports bpp=0, therefore SDL_CalculatePitch()
returns pitch==0, which is then fed to SDL_malloc() (which is malloc())
and malloc(0) returns _something_ which is not NULL but not someting
that we expect..  Then testsprite.c:LoadSprite() accesses the pixels
as *(Uint8*)pixels which valrind reports as:

==15533== Invalid read of size 1
==15533==    at 0x8048C08: LoadSprite (testsprite.c:45)
==15533==    by 0x80492FC: main (testsprite.c:224)
==15533==  Address 0x449e588 is 0 bytes after a block of size 0 alloc'd
==15533==    at 0x40072B2: malloc (vg_replace_malloc.c:270)
==15533==    by 0x4045719: SDL_CreateRGBSurface (SDL_surface.c:126)
==15533==    by 0x40403C1: SDL_LoadBMP_RW (SDL_bmp.c:237)
==15533==    by 0x8048BB2: LoadSprite (testsprite.c:36)
==15533==    by 0x80492FC: main (testsprite.c:224)

Besides, valrind also reports this:
==15533== Conditional jump or move depends on uninitialised value(s)
==15533==    at 0x40403F3: SDL_LoadBMP_RW (SDL_bmp.c:247)
==15533==    by 0x8048BB2: LoadSprite (testsprite.c:36)
==15533==    by 0x80492FC: main (testsprite.c:224)


Easy/quick solution would be early-rejecting a bmp with 0 bpp from
SDL_bmp.c:SDL_LoadBMP_RW().  E.g. something like:
diff --git a/src/video/SDL_bmp.c b/src/video/SDL_bmp.c
--- a/src/video/SDL_bmp.c
+++ b/src/video/SDL_bmp.c
@@ -165,12 +165,13 @@ SDL_Surface * SDL_LoadBMP_RW (SDL_RWops 
 	switch (biBitCount) {
 		case 1:
 		case 4:
 			ExpandBMP = biBitCount;
 			biBitCount = 8;
 			break;
+		case 0:
 		case 2:
 		case 3:
 		case 5:
 		case 6:
 		case 7:
 			SDL_SetError("%d-bpp BMP images are not supported", biBitCount);

Comments?

Fixed, thanks!
https://hg.libsdl.org/SDL/rev/6203d73874ab

The patch seems to make sense to me.


But I can't reproduce the memory issue.
the icon.bmp I got is rejected by SDL_CreateRGBSurface(), 
with error: "Unknown pixel format", but no valgrind leak.

I added some trace before CreateRGBSurface in SDL_LoadBMP:
SDL_Log("surface=%p biWidth=%d biHeight=%d biBitCount=%d Rmask=%08x Gmask=%08x Bmask=%08x Amask=%08x", surface, biWidth, biHeight, biBi    tCount, Rmask, Gmask, Bmask, Amask );

surface=(nil) biWidth=250 biHeight=2560 biBitCount=0 Rmask=00000000 Gmask=00000000 Bmask=00000000 Amask=00000000
Indeed biBitCount is 0, but the surface created is NULL so the function LoadBMP somehow fails



hexdump icon.bmp 
0000000 4d42 ffec ff22 0aff 0000 0000 0000 0000
0000010 ff00 00fa 0000 0a00 0000 0000 0000 0000
0000020 0000                                   
0000022


md5sum icon.bmp 
ad09706f94d97935a3377f0c394018ff  icon.bmp

(In reply to Sylvain from comment #6)
> But I can't reproduce the memory issue.
The issue was reported for SDL1.2 branch.

Fix now applied to SDL-1.2 branch, too:
https://hg.libsdl.org/SDL/rev/13b0e1be3ca2

We're changing how we do SDL release versions; now releases will be even numbers (2.0.10, 2.0.12, etc), and as soon as we tag a release, we'll move the internal version number to an odd number (2.0.12 ships, we tag the latest in revision control as 2.0.13 immediately, which will become 2.0.14 on release, etc).

As such, I'm moving the bugs tagged with target-2.0.11 to target 2.0.12. Sorry if you get a lot of email from this change!

Thanks,
--ryan.

We're changing how we do SDL release versions; now releases will be even numbers (2.0.10, 2.0.12, etc), and as soon as we tag a release, we'll move the internal version number to an odd number (2.0.12 ships, we tag the latest in revision control as 2.0.13 immediately, which will become 2.0.14 on release, etc).

As such, I'm moving the bugs tagged with target-2.0.11 to target 2.0.12. Sorry if you get a lot of email from this change!

Thanks,
--ryan.