Re: bitblt.c - GdiAlphaBlend : Check for NULL Pointer

[Roderick Colenbrander]

Recently Nikolay Sivov wrote some tests for this call. His tests
didn't make it in yet but I think those should enter and then based on
those results changes should be made.

Roderick

On Thu, Sep 10, 2009 at 10:56 PM, Markus Stockhausen
<markus.stockhausen(a)collogia.de> wrote:
> Hi,
>
> http://bugs.winehq.org/show_bug.cgi?id=18041 reveals a bug where
> GdiAlphaBlend will crash in Teamviewer application when a NULL pointer
> is handed over. The attached check fixes that behaviour.
>
> Tanks in advance.
>
> Markus

bitblt.c - GdiAlphaBlend : Check for NULL Pointer

[Nikolay Sivov]

Roderick Colenbrander wrote:
> Recently Nikolay Sivov wrote some tests for this call. His tests
> didn't make it in yet but I think those should enter and then based on
> those results changes should be made.
>
> Roderick
>
> On Thu, Sep 10, 2009 at 10:56 PM, Markus Stockhausen
> <markus.stockhausen(a)collogia.de> wrote:
> > Hi,
> >
> > http://bugs.winehq.org/show_bug.cgi?id=18041 reveals a bug where
> > GdiAlphaBlend will crash in Teamviewer application when a NULL pointer
> > is handed over. The attached check fixes that behaviour.
> >
> > Tanks in advance.
> >
> > Markus
>
> This is what Roderick is talking about:

http://www.winehq.org/pipermail/wine-patches/2009-September/078293.html

I don't think it will be committed, it should be included with a fix.

You may take this part and resubmit:
---
+ SetLastError(0xdeadbeef);
+ expect_eq(pGdiAlphaBlend(hdcDst, 0, 0, 20, 20, NULL, 0, 0, 20, 20, 
blend), FALSE, BOOL, "%d");
+ expect_eq(GetLastError(), 0xdeadbeef, int, "%d");
---

bitblt.c - GdiAlphaBlend : Check for NULL Pointer

[Markus Stockhausen]

Am Freitag, den 11.09.2009, 01:17 +0400 schrieb Nikolay Sivov:
> Roderick Colenbrander wrote:
> > Recently Nikolay Sivov wrote some tests for this call. His tests
> > didn't make it in yet but I think those should enter and then based on
> > those results changes should be made.
> >
> > Roderick
> >
> > On Thu, Sep 10, 2009 at 10:56 PM, Markus Stockhausen
> > <markus.stockhausen(a)collogia.de> wrote:
> > > Hi,
> > >
> > > http://bugs.winehq.org/show_bug.cgi?id=18041 reveals a bug where
> > > GdiAlphaBlend will crash in Teamviewer application when a NULL pointer
> > > is handed over. The attached check fixes that behaviour.
> > >
> > > Tanks in advance.
> > >
> > > Markus
> >
> > This is what Roderick is talking about:
>
> http://www.winehq.org/pipermail/wine-patches/2009-September/078293.html
>
> I don't think it will be committed, it should be included with a fix.
>
> You may take this part and resubmit:
> ---
> + SetLastError(0xdeadbeef);
> + expect_eq(pGdiAlphaBlend(hdcDst, 0, 0, 20, 20, NULL, 0, 0, 20, 20, 
> blend), FALSE, BOOL, "%d");
> + expect_eq(GetLastError(), 0xdeadbeef, int, "%d");
> ---

Phew, in 3 days from 0 to 100. Too much for me. If I get it right, there
are two things to do.

1. I should optimize my patch and remove unnecessary dcSrc checks.
2. A testcase is needed to prove the correct function behaviour in
Windows and Wine. The tastcase is already available but not commited
yet.

For me this means:

1. Wait for testcase implementation.
2. Create new patch and test under Wine
3. Ask someone to do test under Windows
2. Resent patch to mailing list.

Thanks for your patience.

Markus

bitblt.c - GdiAlphaBlend : Check for NULL Pointer

[Nikolay Sivov]

Markus Stockhausen wrote:
> Am Freitag, den 11.09.2009, 01:17 +0400 schrieb Nikolay Sivov:
> >
>
> Phew, in 3 days from 0 to 100. Too much for me. If I get it right, there
> are two things to do.
>
> 1. I should optimize my patch and remove unnecessary dcSrc checks.

Actually, yes. After your patch this check:
---
if (dcSrc) update_dc( dcSrc );
---
is useless.
> 2. A testcase is needed to prove the correct function behaviour in
> Windows and Wine. The tastcase is already available but not commited
> yet.

Yes, testcase is trivial. I also did some visual tests and saw now 
difference on destination
dc calling this with source == NULL.
> For me this means:
>
> 1. Wait for testcase implementation.
> 2. Create new patch and test under Wine
> 3. Ask someone to do test under Windows
> 2. Resent patch to mailing list.
>
> Thanks for your patience.
>
> Markus

Just correct your patch (as you mentioned before), include simple 
testcase and resend.
So nothing to wait for.

Basic test shows it doesn't crash on Windows and doesn't change last 
error - I already
check this on XP and it passes.

bitblt.c - GdiAlphaBlend : Check for NULL Pointer

[Markus Stockhausen]

Am Freitag, den 11.09.2009, 01:56 +0400 schrieb Nikolay Sivov:
> Just correct your patch (as you mentioned before), include simple 
> testcase and resend.
> So nothing to wait for.
>
> Basic test shows it doesn't crash on Windows and doesn't change last 
> error - I already
> check this on XP and it passes.

Ok, attached you will find my second try. Testcase of Nicolay included
this time. Hopefully it will be better this time.

Best regards.

bitblt.c - GdiAlphaBlend : Check for NULL Pointer

[Paul Vriens]

On 09/11/2009 12:47 AM, Markus Stockhausen wrote:
> Am Freitag, den 11.09.2009, 01:56 +0400 schrieb Nikolay Sivov:
> > Just correct your patch (as you mentioned before), include simple
> > testcase and resend.
> > So nothing to wait for.
> >
> > Basic test shows it doesn't crash on Windows and doesn't change last
> > error - I already
> > check this on XP and it passes.
>
> Ok, attached you will find my second try. Testcase of Nicolay included
> this time. Hopefully it will be better this time.
>
> Best regards.
>
> ------------------------------------------------------------------------

Hi Markus,

One I see is a style issue with the if statement. There shouldn't be a 
space after the opening and before the closing bracket.

Another thing (style again) is that maybe it's nicer to return FALSE 
instead of ret (as is done throughout this file).

-- 
Cheers,

Paul.

bitblt.c - GdiAlphaBlend : Check for NULL Pointer

[Markus Stockhausen]

Am Freitag, den 11.09.2009, 09:04 +0200 schrieb Paul Vriens:
> Hi Markus,
>
> One I see is a style issue with the if statement. There shouldn't be a 
> space after the opening and before the closing bracket.
>
> Another thing (style again) is that maybe it's nicer to return FALSE 
> instead of ret (as is done throughout this file).

Resending the desired changes and hoping to see them finding their way
into git ...

Best regards.

bitblt.c - GdiAlphaBlend : Check for NULL Pointer

[Paul Vriens]

On 09/11/2009 09:13 AM, Markus Stockhausen wrote:
> Am Freitag, den 11.09.2009, 09:04 +0200 schrieb Paul Vriens:
> > Hi Markus,
> >
> > One I see is a style issue with the if statement. There shouldn't be a
> > space after the opening and before the closing bracket.
> >
> > Another thing (style again) is that maybe it's nicer to return FALSE
> > instead of ret (as is done throughout this file).
>
> Resending the desired changes and hoping to see them finding their way
> into git ...
>
> Best regards.

Patches that you want to have committed should go to wine-patches.

I did found another small issue though:

line 532: "dcSrc ? dcSrc->physDev : NULL" this should be replaced as well.

-- 
Cheers,

Paul.