Re: gdi32/bitblt: GdiAlphaBlend - Check for NULL Pointer (incl. testcase)
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. -- Cheers, Paul.
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 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). 2. Check if dcSrc is not NULL (that is missing) I'm a little bit lost if more testcases are needed. Best regards.
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.
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.
participants (2)
-
Markus Stockhausen -
Paul Vriens