Re: [2/5] d3d9: Test invalid buffer lock parameters

[Henri Verbeet]

On 29 January 2010 11:35, Stefan Dösinger  wrote:
> +static HMODULE d3d9_handle = 0;

This is redundant.
> - hr = IDirect3DVertexBuffer9_Lock(buffer, 0, 0, (void *) &bufstart, 0);
> - ok(hr == D3D_OK, "IDirect3DVertexBuffer9_Lock failed, 0x%08x\n", hr);
> - hr = IDirect3DVertexBuffer9_Unlock(buffer);
> - ok(hr == D3D_OK, "IDirect3DVertexBuffer9_Unlock failed, 0x%08x\n", hr);
> -
> - hr = IDirect3DVertexBuffer9_Lock(buffer, 0, 1024, (void *) &data, 0);
> - ok(hr == D3D_OK, "IDirect3DVertexBuffer9_Lock failed, 0x%08x\n", hr);
> - hr = IDirect3DVertexBuffer9_Unlock(buffer);
> - ok(hr == D3D_OK, "IDirect3DVertexBuffer9_Unlock failed, 0x%08x\n", hr);
> - ok(bufstart == data, "Two locks returned different addresses: %p, %p\n", bufstart, data);

Why would we care if the buffer is always mapped at the same address?
Also note that the test doesn't necessarily prove this is the case.
> - /* This happily wraps around the address space */
> - hr = IDirect3DVertexBuffer9_Lock(buffer, ~0U, 1024, (void *) &data, 0);
> - ok(hr == D3D_OK, "IDirect3DVertexBuffer9_Lock(start ~0U) failed, 0x%08x\n", hr);
> - hr = IDirect3DVertexBuffer9_Unlock(buffer);
> - ok(hr == D3D_OK, "IDirect3DVertexBuffer9_Unlock failed, 0x%08x\n", hr);
> - ok(bufstart -1 == data, "~0U start lock returned unexpected address: expected %p, got %p\n", bufstart - 1, data);

I'd expect this to break on 64 bit. Also, like the previous test, why
do we care? Do you seriously have an application that does this?

[2/5] d3d9: Test invalid buffer lock parameters

[Stefan Dösinger]

On Friday 29 January 2010 12:21:27 Henri Verbeet wrote:
> On 29 January 2010 11:35, Stefan Dösinger  wrote:
> > +static HMODULE d3d9_handle = 0;
>
> This is redundant.
> > - hr = IDirect3DVertexBuffer9_Lock(buffer, 0, 0, (void *) &bufstart,
> >
> > 0); +    ok(hr == D3D_OK, "IDirect3DVertexBuffer9_Lock failed, 0x%08x\n",
> > hr); +    hr = IDirect3DVertexBuffer9_Unlock(buffer);
> > - ok(hr == D3D_OK, "IDirect3DVertexBuffer9_Unlock failed, 0x%08x\n",
> >
> > hr); +
> > - hr = IDirect3DVertexBuffer9_Lock(buffer, 0, 1024, (void *) &data,
> >
> > 0); +    ok(hr == D3D_OK, "IDirect3DVertexBuffer9_Lock failed, 0x%08x\n",
> > hr); +    hr = IDirect3DVertexBuffer9_Unlock(buffer);
> > - ok(hr == D3D_OK, "IDirect3DVertexBuffer9_Unlock failed, 0x%08x\n",
> >
> > hr); +    ok(bufstart == data, "Two locks returned different addresses:
> > %p, %p\n", bufstart, data);
>
> Why would we care if the buffer is always mapped at the same address?
> Also note that the test doesn't necessarily prove this is the case.

It's more like a sanity check since all the other tests testing the handling 
of invalid offsets implicitly make the same assumption. I could make this an 
if(...) skip(); instead of an ERR.

As for why we care: We don't really care that it's this specific result, since 
I don't know any app that depends on this. I mainly want to test the return 
value, and secondarily show that d3d doesn't in some way 'sanitize' the 
range(e.g. set offset=0, length=size), but instead of trying to show what is 
not returned the test just shows that the normal math from the non-error cases 
is applied.
> > - /* This happily wraps around the address space */
> > - hr = IDirect3DVertexBuffer9_Lock(buffer, ~0U, 1024, (void *) &data,
> >
> > 0); +    ok(hr == D3D_OK, "IDirect3DVertexBuffer9_Lock(start ~0U) failed,
> > 0x%08x\n", hr); +    hr = IDirect3DVertexBuffer9_Unlock(buffer);
> > - ok(hr == D3D_OK, "IDirect3DVertexBuffer9_Unlock failed, 0x%08x\n",
> >
> > hr); +    ok(bufstart -1 == data, "~0U start lock returned unexpected
> > address: expected %p, got %p\n", bufstart - 1, data);
>
> I'd expect this to break on 64 bit. Also, like the previous test, why
> do we care? Do you seriously have an application that does this?

True, it'll break, I'll remove the address check(but keep the hr == D3D_OK 
check).