DDRAW: Surface locking patch, take 2

[Matthew Mastracci]

Christian Costa wrote:

> Hi Matthew,
> 
> This seems fine.
> There are some comments though, see below.
>
...
>> +    if (src == iface) {
>> +        int pitch;
>> +
>> +        UnionRect(&lock_union, &lock_src, &lock_dst);
>> +
>> +        /* Lock the union of the two rectangles */
>> +        IDirectDrawSurface7_Lock(iface, &lock_union, &ddesc, 0, 0);
>> +
>> +        pitch = This->surface_desc.u1.lPitch;
>> +
>>
> ddesc has not been properly initialized.
> you should also copy sdesc from ddesc.

It actually has.  If you look higher up in the function, ddesc and sdesc 
are copied from This->surface_desc:

     /* Get the surface description without locking to first compute the 
width / height */
     ddesc = This->surface_desc;
     sdesc = (ICOM_OBJECT(IDirectDrawSurfaceImpl, IDirectDrawSurface7, 
src))->surface_desc;

AFAICT, ddesc and sdesc will always be valid after this initialization.

>> +    if (src == iface) {
>> +        IDirectDrawSurface7_Unlock(iface, &lock_union);
>> +    } else {
>> +        IDirectDrawSurface7_Unlock(iface, &lock_dst);
>> +        IDirectDrawSurface7_Unlock(src, &lock_src);
>>
> You should check if src != NULL before trying to unlocking it.

BltFast() doesn't accept a NULL src, so this isn't necessary.  The 
function should probably check for NULL src before the surface lock 
check.  If I can figure out what the error return code should be (I 
think it's DDERR_INVALIDPARAM), I can check for a NULL src at the start 
of the function in a later patch.

Thanks for the review,
Matt.