Re: [PATCH] user32: honor bfOffBits in BITMAP_Load
Wolfram Sang <wolfram(a)the-dreams.de> wrote:
+ offbits = bmfh->bfOffBits - sizeof(BITMAPFILEHEADER);
+ bits = (char *)info + (offbits ?: size);
The purpose of offbits is not clear. Besides "?:" is absolutely not readable, putting an explicit 0 there would help a bit. Adding a test case wouldn't hurt either. -- Dmitry.
Dmitry Timoshkov wrote:
Wolfram Sang <wolfram(a)the-dreams.de> wrote:
+ offbits = bmfh->bfOffBits - sizeof(BITMAPFILEHEADER);
+ bits = (char *)info + (offbits ?: size);
The purpose of offbits is not clear.
It has the same purpose as 'offbits' in BmpFrameDecode_ReadUncompressed() from bmpdecode.c. And in bfOffBits, too. Please give me a hint, I still think it is clear this way.
Besides "?:" is absolutely not readable, putting an explicit 0 there would help a bit.
:D Okay, that proves your point. This is a short from for 'offbits ? offbits : size'. Will fix that.
Adding a test case wouldn't hurt either.
Hmm, do you have a pointer for a test verifying how something should look like (in contrast to checking a return value)? Was this not necessary for LoadImage() in general? Regards, Wolfram
On 6 April 2010 16:42, Wolfram Sang <wolfram(a)the-dreams.de> wrote:
Besides "?:" is absolutely not readable, putting an explicit 0 there would help a bit.
:D Okay, that proves your point. This is a short from for 'offbits ? offbits : size'. Will fix that.
I also think that's a gcc extension, in general Wine tries to stick to C89.
Wolfram Sang <wolfram(a)the-dreams.de> wrote:
Hmm, do you have a pointer for a test verifying how something should look like (in contrast to checking a return value)? Was this not necessary for LoadImage() in general?
Have a look at gdi32 bitmap tests. -- Dmitry.
participants (3)
-
Dmitry Timoshkov -
Henri Verbeet -
Wolfram Sang