Re: [PATCH 1/4] ddraw: Don't write out if a ddraw1 surface of size 0 is requested.

[Henri Verbeet]

On 6 January 2017 at 08:07, Alex Henrie <alexhenrie24(a)gmail.com> wrote:
> Fixes https://bugs.winehq.org/show_bug.cgi?id=42159

This isn't a regression, right?
> @@ -2975,10 +2975,7 @@ static HRESULT WINAPI ddraw1_CreateSurface(IDirectDraw *iface,
>      hr = ddraw_surface_create(ddraw, &surface_desc2, &impl, outer_unknown, 1);
>      wined3d_mutex_unlock();
>      if (FAILED(hr))
> -    {
> -        *surface = NULL;
>          return hr;
> -    }
> This does a bit more than the commit message says it does, this
> doesn't write to *surface on any error from ddraw_surface_create().
> That may very well be correct, but that's not what the test tests. The
> test only tests that CreateSurface() doesn't crash on error with an
> invalid "surface" pointer, but that could also be because of e.g. an
> exception handler. If you want to prove that it doesn't (try to) write
> to the "surface" pointer, you should pass a valid pointer to
> CreateSurface() and test that it isn't modified after the call.

[PATCH 1/4] ddraw: Don't write out if a ddraw1 surface of size 0 is requested.

[Alex Henrie]

2017-01-06 4:57 GMT-07:00 Henri Verbeet <hverbeet(a)gmail.com>:
> On 6 January 2017 at 08:07, Alex Henrie <alexhenrie24(a)gmail.com> wrote:
> > Fixes https://bugs.winehq.org/show_bug.cgi?id=42159
>
> This isn't a regression, right?
> > @@ -2975,10 +2975,7 @@ static HRESULT WINAPI ddraw1_CreateSurface(IDirectDraw *iface,
> >      hr = ddraw_surface_create(ddraw, &surface_desc2, &impl, outer_unknown, 1);
> >      wined3d_mutex_unlock();
> >      if (FAILED(hr))
> > -    {
> > -        *surface = NULL;
> >          return hr;
> > -    }
> > This does a bit more than the commit message says it does, this
> > doesn't write to *surface on any error from ddraw_surface_create().
> > That may very well be correct, but that's not what the test tests. The
> > test only tests that CreateSurface() doesn't crash on error with an
> > invalid "surface" pointer, but that could also be because of e.g. an
> > exception handler. If you want to prove that it doesn't (try to) write
> > to the "surface" pointer, you should pass a valid pointer to
> > CreateSurface() and test that it isn't modified after the call.

Thanks for the feedback. No, this isn't a regression. Should I wait
until after the code freeze to submit a revised patchset?

-Alex

[PATCH 1/4] ddraw: Don't write out if a ddraw1 surface of size 0 is requested.

[Henri Verbeet]

On 6 January 2017 at 19:31, Alex Henrie <alexhenrie24(a)gmail.com> wrote:
> Thanks for the feedback. No, this isn't a regression. Should I wait
> until after the code freeze to submit a revised patchset?

Submitting it is fine, and I'll review it, but I won't sign off on it
until after the release.