Re: [PATCH] ddraw: Return a nullpointer as lpSurface in Lock() if the rect is invalid.

[Stefan Dösinger]

Am Mittwoch, 26. Dezember 2007 22:35:16 schrieb Günther Brammer:
> -
>        memset(&locked_desc, 1, sizeof(locked_desc));
> -
>        locked_desc.dwSize = sizeof(locked_desc);
> -
>        hr = IDirectDrawSurface_Lock(surface, rect, &locked_desc,
>
> DDLOCK_WAIT, NULL); ok(hr == DDERR_INVALIDPARAMS, "Lock returned 0x%08x for
> rect [%d, %d]->[%d, %d]" ", expected DDERR_INVALIDPARAMS (0x%08x)\n", hr,
> rect->left, rect->top, rect->right, rect->bottom, DDERR_INVALIDPARAMS); +  
>          ok(!memcmp(&locked_desc, &surface_desc, sizeof(locked_desc)),
> "IDirectDrawSurface_Lock did not set lpSurface in the surface desc\n"); }

I think you should just compare lpSurface instead of memcmp'ing the entire 
structure.

Also, in the patch that changes surface.c, it seems that the patch changes a 
few whitespaces(unless I missed something).

[PATCH] ddraw: Return a nullpointer as lpSurface in Lock() if the rect is invalid.

[Günther Brammer]

Stefan Dösinger <stefan <at> codeweavers.com> writes:
> Am Mittwoch, 26. Dezember 2007 22:35:16 schrieb Günther Brammer:
> > -
> >        memset(&locked_desc, 1, sizeof(locked_desc));
> > -
> >        locked_desc.dwSize = sizeof(locked_desc);
> > -
> >        hr = IDirectDrawSurface_Lock(surface, rect, &locked_desc,
> >
> > DDLOCK_WAIT, NULL); ok(hr == DDERR_INVALIDPARAMS, "Lock returned 0x%08x for
> > rect [%d, %d]->[%d, %d]" ", expected DDERR_INVALIDPARAMS (0x%08x)\n", hr,
> > rect->left, rect->top, rect->right, rect->bottom, DDERR_INVALIDPARAMS); +  
> >          ok(!memcmp(&locked_desc, &surface_desc, sizeof(locked_desc)),
> > "IDirectDrawSurface_Lock did not set lpSurface in the surface desc\n"); }
>
> I think you should just compare lpSurface instead of memcmp'ing the entire 
> structure.

I was told on IRC to "test as much as you can get tested", and
memcmp'ing the entire structure made me change my patch. Initially I had
Lock fill the entire structure as it did in Wine 9.35. But ok, Baldurs
Gate certainly only cares about lpSurface.
> Also, in the patch that changes surface.c, it seems that the patch changes a 
> few whitespaces(unless I missed something).

It does not change existing whitespace but adds two empty lines with
trailing whitespace. I'll resend without that.

Günther