gdi32/bitblt: GdiAlphaBlend - Check for NULL Pointer (incl. testcase)

[Paul Vriens]

On 09/18/2009 02:31 PM, Markus Stockhausen wrote:
> Am Freitag, den 18.09.2009, 14:09 +0200 schrieb Paul Vriens:
>> On 09/18/2009 01:42 PM, Markus Stockhausen wrote:
>>> Hi,
>>>
>>> as I was encouraged to resent the patch from last week in git format I
>>> hope it will be better this time.
>>>
>>> Best regards
>>>
>>>
>>> ------------------------------------------------------------------------
>>>
>>>
>> Hi Markus,
>>
>> After reading this patch again, it seems to me it's not a 100% correct.
>> There is a slight difference in passing hdcSrc as NULL compared to
>> retrieving dcSrc via get_dc_ptr.
>>
>> The actual test sets hdcSrc to NULL, so the corresponding test in the
>> implementation should be:
>>
>> if (!hdcSrc)
>>       return FALSE;
>>
>> dcSrc = get_dc_ptr ( hdcSrc );
>>
>> Now, I don't know whether get_dc_ptr can fail but that's a different matter.
>>
>
> Hi Paul,
>
> thanks for your attention but if I go through the whole dc_* stuff I can
> see that it will return NULL to dcSrc if an error occurs somewhere. This

But if that's the case the test wouldn't crash.

> maybe because auf hdcSrc being NULL (other cases included). This would
> result in two cases to check:
>
> 1. Check if hdcSrc is not NULL (will be done in the dc_ functions).

As said, then the test shouldn't crash.

> 2. Check if dcSrc is not NULL (that is missing)

This piece is not missing as dcSrc is checked for in the rest of 
GdiAlphaBlend.
>
> I'm a little bit lost if more testcases are needed.
>

It seems to me that the crash happens somewhere else and not in 
GdiAlphaBlend.

I still think is better to check for hdcSrc being NULL at the beginning 
of the GdiAlphaBlend. The only thing we now from the test is that the 
last errors in not changed when we pass a NULL hdcSrc. All other 
possibile error conditions will be hard to tackle/test, I guess.

I see that we have loads of functions that expect a hdcSrc, btw.

-- 
Cheers,

Paul.