Re: [PATCH] gdi32/tests: Remove a useless test

[Detlef Riekenberg]

Dimitry wrote:
> > --- a/dlls/gdi32/tests/dc.c
> > +++ b/dlls/gdi32/tests/dc.c
> > @@ -815,9 +815,6 @@ static void test_DeleteDC(void)
> >      ret = UnregisterClassA("Wine class DC", GetModuleHandleA(NULL));
> >      ok(ret, "UnregisterClassA failed\n");
> > - ret = GetObjectType(hdc_test);
> > - ok(!ret, "GetObjectType should fail for a deleted DC\n");
>
> It should be changed to add broken() instead of removing a perfectly
> > valid test.

No. The test is a "Use after free" Bug

The result of the function-call is an implementation Detail
and can change with every Update.
Microsoft changed the function-result more than a year ago...

There  are more 'Use after free' bugs in this file,
but the others did not produce test failures yet.

--
Bye bye ...
         ... Detlef

[PATCH] gdi32/tests: Remove a useless test

[Dmitry Timoshkov]

"Detlef Riekenberg"  wrote:
> > > --- a/dlls/gdi32/tests/dc.c
> > > +++ b/dlls/gdi32/tests/dc.c
> > > @@ -815,9 +815,6 @@ static void test_DeleteDC(void)
> > >      ret = UnregisterClassA("Wine class DC", GetModuleHandleA(NULL));
> > >      ok(ret, "UnregisterClassA failed\n");
> > > - ret = GetObjectType(hdc_test);
> > > - ok(!ret, "GetObjectType should fail for a deleted DC\n");
> >
> > It should be changed to add broken() instead of removing a perfectly
> > > valid test.
>
> No. The test is a "Use after free" Bug
>
> The result of the function-call is an implementation Detail
> and can change with every Update.
> Microsoft changed the function-result more than a year ago...
>
> There  are more 'Use after free' bugs in this file,
> but the others did not produce test failures yet.

If by "use after free" you mean to return success for random trash
the API gets passed in then it's clearly broken behaviour.

[PATCH] gdi32/tests: Remove a useless test

[Alexandre Julliard]

Dmitry Timoshkov  writes:
> "Detlef Riekenberg"  wrote:
> > > > --- a/dlls/gdi32/tests/dc.c
> > > > +++ b/dlls/gdi32/tests/dc.c
> > > > @@ -815,9 +815,6 @@ static void test_DeleteDC(void)
> > > >      ret = UnregisterClassA("Wine class DC", GetModuleHandleA(NULL));
> > > >      ok(ret, "UnregisterClassA failed\n");
> > > > - ret = GetObjectType(hdc_test);
> > > > - ok(!ret, "GetObjectType should fail for a deleted DC\n");
> > >
> > > It should be changed to add broken() instead of removing a perfectly
> > > > valid test.
> >
> > No. The test is a "Use after free" Bug
> >
> > The result of the function-call is an implementation Detail
> > and can change with every Update.
> > Microsoft changed the function-result more than a year ago...
> >
> > There  are more 'Use after free' bugs in this file,
> > but the others did not produce test failures yet.
>
> If by "use after free" you mean to return success for random trash
> the API gets passed in then it's clearly broken behaviour.

If the DC was actually deleted, succeeding is clearly broken. However,
it would be reasonable for instance to reuse the DC as cached DC instead
of deleting it. IIRC our implementation was doing that at some point.
This may need more tests to figure out exactly what Windows is doing
with the DC.