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

[Paul Vriens]

On 09/18/2009 02:50 PM, Paul Vriens wrote:
> 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.
>

Crashes in X11DRV_AlphaBlend, btw.

-- 
Cheers,

Paul.