Re: [PATCH] user32: honor bfOffBits in BITMAP_Load

[Dmitry Timoshkov]

Wolfram Sang  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.

[PATCH] user32: honor bfOffBits in BITMAP_Load

[Wolfram Sang]

Dmitry Timoshkov wrote:
> Wolfram Sang  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

[PATCH] user32: honor bfOffBits in BITMAP_Load

[Henri Verbeet]

On 6 April 2010 16:42, Wolfram Sang  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.

[PATCH] user32: honor bfOffBits in BITMAP_Load

[Dmitry Timoshkov]

Wolfram Sang  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.