Re: D3D8_GetCreationParameters fix

[Mike McCormack]

You probably want to use a Windows method to allocate memory here, such 
as HeapAlloc() or CoTaskMemAlloc, as the caller is not going to use 
free() to free the memory...

Mike

Andrei Barbu wrote:
> Changelog:
>
> Fixes a memory access fault inside of D3D8_GetCreationParameters.
> ...
>  HRESULT  WINAPI  IDirect3DDevice8Impl_GetCreationParameters(LPDIRECT3DDEVICE8 iface, D3DDEVICE_CREATION_PARAMETERS *pParameters) {
>      ICOM_THIS(IDirect3DDevice8Impl,iface);
>      TRACE("(%p) copying to %p\n", This, pParameters);    
> +
> +    pParameters = (D3DDEVICE_CREATION_PARAMETERS*) malloc( sizeof( D3DDEVICE_CREATION_PARAMETERS ));
>      memcpy(pParameters, &This->CreateParms, sizeof(D3DDEVICE_CREATION_PARAMETERS));
>      return D3D_OK;
>  }

D3D8_GetCreationParameters fix

[Marcus Meissner]

On Fri, Jul 09, 2004 at 04:39:31AM +0900, Mike McCormack wrote:
> You probably want to use a Windows method to allocate memory here, such 
> as HeapAlloc() or CoTaskMemAlloc, as the caller is not going to use 
> free() to free the memory...
> Mike
>
> Andrei Barbu wrote:
> > Changelog:
> >
> > Fixes a memory access fault inside of D3D8_GetCreationParameters.
> > ...
> > HRESULT  WINAPI  
> > IDirect3DDevice8Impl_GetCreationParameters(LPDIRECT3DDEVICE8 iface, 
> > D3DDEVICE_CREATION_PARAMETERS *pParameters) {
> >     ICOM_THIS(IDirect3DDevice8Impl,iface);
> >     TRACE("(%p) copying to %p\n", This, pParameters);    
> > +
> > +    pParameters = (D3DDEVICE_CREATION_PARAMETERS*) malloc( sizeof( 
> > D3DDEVICE_CREATION_PARAMETERS ));
> >     memcpy(pParameters, &This->CreateParms, 
> >     sizeof(D3DDEVICE_CREATION_PARAMETERS));
> >     return D3D_OK;
> > }

Its also wrong, because the patch just overwrites the pParameters argument
and does not return the allocated memory.

Ciao, Marcus

D3D8_GetCreationParameters fix

[Andrei Barbu]

It doesn't have to return that memory, DX functions work by taking
parameters they modify. In essence, that's the point, modifying
pParameters and returning D3D_OK

Andrei

On Thu, 2004-07-08 at 15:50, Marcus Meissner wrote:
> On Fri, Jul 09, 2004 at 04:39:31AM +0900, Mike McCormack wrote:
> > You probably want to use a Windows method to allocate memory here, such 
> > as HeapAlloc() or CoTaskMemAlloc, as the caller is not going to use 
> > free() to free the memory...
> > Mike
> >
> > Andrei Barbu wrote:
> > > Changelog:
> > >
> > > Fixes a memory access fault inside of D3D8_GetCreationParameters.
> > > ...
> > > HRESULT  WINAPI  
> > > IDirect3DDevice8Impl_GetCreationParameters(LPDIRECT3DDEVICE8 iface, 
> > > D3DDEVICE_CREATION_PARAMETERS *pParameters) {
> > >     ICOM_THIS(IDirect3DDevice8Impl,iface);
> > >     TRACE("(%p) copying to %p\n", This, pParameters);    
> > > +
> > > +    pParameters = (D3DDEVICE_CREATION_PARAMETERS*) malloc( sizeof( 
> > > D3DDEVICE_CREATION_PARAMETERS ));
> > >     memcpy(pParameters, &This->CreateParms, 
> > >     sizeof(D3DDEVICE_CREATION_PARAMETERS));
> > >     return D3D_OK;
> > > }
>
> Its also wrong, because the patch just overwrites the pParameters argument
> and does not return the allocated memory.
>
> Ciao, Marcus

D3D8_GetCreationParameters fix

[Mike Hearn]

On Thu, 08 Jul 2004 18:58:57 -0400, Andrei Barbu wrote:
> It doesn't have to return that memory, DX functions work by taking
> parameters they modify. In essence, that's the point, modifying
> pParameters and returning D3D_OK

Right, what he means is that the parameter is basically a pointer to a
pointer, so you have this:

HRESULT SomeFunc(void **output) {
	
	output = HeapAlloc(GetProcessHeap(), 0, sizeof(whatever)); 
	// that is wrong, as you are just modifying a local argument. 
	// the pointer to the new memory will be lost when the function exits

	*output = HeapAlloc(GetProcessHeap(), 0, sizeof(whatever));
	// that's right, as you're now assigning to the location pointed
	// to by output

}

thanks -mike

D3D8_GetCreationParameters fix

[Christian Costa]

Andrei Barbu wrote:
> It doesn't have to return that memory, DX functions work by taking
> parameters they modify. In essence, that's the point, modifying
> pParameters and returning D3D_OK

Marcus is right.
Morover, according to the doc, a pointer to a valid area must be passed 
as parameter (the prototype confirms that).
BTW, what is the problem you intend to fix?
> Andrei
>
> On Thu, 2004-07-08 at 15:50, Marcus Meissner wrote:
> > On Fri, Jul 09, 2004 at 04:39:31AM +0900, Mike McCormack wrote:
> > > You probably want to use a Windows method to allocate memory here, such 
> > > as HeapAlloc() or CoTaskMemAlloc, as the caller is not going to use 
> > > free() to free the memory...
> > >
> > > Mike
> > >
> > > Andrei Barbu wrote:
> > > > Changelog:
> > > >
> > > > Fixes a memory access fault inside of D3D8_GetCreationParameters.
> > >
> > > ...
> > > > HRESULT  WINAPI  
> > > > IDirect3DDevice8Impl_GetCreationParameters(LPDIRECT3DDEVICE8 iface, 
> > > > D3DDEVICE_CREATION_PARAMETERS *pParameters) {
> > > >    ICOM_THIS(IDirect3DDevice8Impl,iface);
> > > >    TRACE("(%p) copying to %p\n", This, pParameters);    
> > > > +
> > > > +    pParameters = (D3DDEVICE_CREATION_PARAMETERS*) malloc( sizeof( 
> > > > D3DDEVICE_CREATION_PARAMETERS ));
> > > >    memcpy(pParameters, &This->CreateParms, 
> > > >    sizeof(D3DDEVICE_CREATION_PARAMETERS));
> > > >    return D3D_OK;
> > > > }
> >
> > Its also wrong, because the patch just overwrites the pParameters argument
> > and does not return the allocated memory.
> >
> > Ciao, Marcus

D3D8_GetCreationParameters fix

[Andrei Barbu]

I agree on the docs, interestingly enough though,
Chessmaster 9000 gives an invalid pointer (0x1)
and there's a memory access error.

Works on Windows though, so I'm assuming DX might be
allocating the memory on it's own.

On Thu, 2004-07-08 at 19:45, Christian Costa wrote:
> Andrei Barbu wrote:
> > It doesn't have to return that memory, DX functions work by taking
> > parameters they modify. In essence, that's the point, modifying
> > pParameters and returning D3D_OK
>
> Marcus is right.
> Morover, according to the doc, a pointer to a valid area must be passed 
> as parameter (the prototype confirms that).
> BTW, what is the problem you intend to fix?
> > Andrei
> >
> > On Thu, 2004-07-08 at 15:50, Marcus Meissner wrote:
> > > On Fri, Jul 09, 2004 at 04:39:31AM +0900, Mike McCormack wrote:
> > > > You probably want to use a Windows method to allocate memory here, such 
> > > > as HeapAlloc() or CoTaskMemAlloc, as the caller is not going to use 
> > > > free() to free the memory...
> > > >
> > > > Mike
> > > >
> > > > Andrei Barbu wrote:
> > > > > Changelog:
> > > > >
> > > > > Fixes a memory access fault inside of D3D8_GetCreationParameters.
> > > >
> > > > ...
> > > > > HRESULT  WINAPI  
> > > > > IDirect3DDevice8Impl_GetCreationParameters(LPDIRECT3DDEVICE8 iface, 
> > > > > D3DDEVICE_CREATION_PARAMETERS *pParameters) {
> > > > >    ICOM_THIS(IDirect3DDevice8Impl,iface);
> > > > >    TRACE("(%p) copying to %p\n", This, pParameters);    
> > > > > +
> > > > > +    pParameters = (D3DDEVICE_CREATION_PARAMETERS*) malloc( sizeof( 
> > > > > D3DDEVICE_CREATION_PARAMETERS ));
> > > > >    memcpy(pParameters, &This->CreateParms, 
> > > > >    sizeof(D3DDEVICE_CREATION_PARAMETERS));
> > > > >    return D3D_OK;
> > > > > }
> > >
> > > Its also wrong, because the patch just overwrites the pParameters argument
> > > and does not return the allocated memory.
> > >
> > > Ciao, Marcus

D3D8_GetCreationParameters fix

[Lionel Ulmer]

On Thu, Jul 08, 2004 at 08:45:16PM -0400, Andrei Barbu wrote:
> I agree on the docs, interestingly enough though,
> Chessmaster 9000 gives an invalid pointer (0x1)
> and there's a memory access error.
>
> Works on Windows though, so I'm assuming DX might be
> allocating the memory on it's own.

When you mean 'works on Windows' is it Chessmaser 9000 or the fact that
giving a bad pointer to this DX8 API call does not make the program crash ?

Because to be sure to get your patch applied, the best way would be to
understand WHY CM9000 gives this 0x1 pointer to the DX8 function. It may be
due to a memory allocation error, some stack trashing, some other bugs in
Wine, ...

So I fear that only doing some REing of the CM9000 code would help here.


             Lionel

-- 
		 Lionel Ulmer - http://www.bbrox.org/

D3D8_GetCreationParameters fix

[Andrei Barbu]

Having given thought to the issue, you're right.
That was a bad solution, sorry.

It should have failed with D3DERR_INVALIDCALL if it's a bad write
pointer.
Is this new patch ok?

On Thu, 2004-07-08 at 19:45, Christian Costa wrote:
> Andrei Barbu wrote:
> > It doesn't have to return that memory, DX functions work by taking
> > parameters they modify. In essence, that's the point, modifying
> > pParameters and returning D3D_OK
>
> Marcus is right.
> Morover, according to the doc, a pointer to a valid area must be passed 
> as parameter (the prototype confirms that).
> BTW, what is the problem you intend to fix?
> > Andrei
> >
> > On Thu, 2004-07-08 at 15:50, Marcus Meissner wrote:
> > > On Fri, Jul 09, 2004 at 04:39:31AM +0900, Mike McCormack wrote:
> > > > You probably want to use a Windows method to allocate memory here, such 
> > > > as HeapAlloc() or CoTaskMemAlloc, as the caller is not going to use 
> > > > free() to free the memory...
> > > >
> > > > Mike
> > > >
> > > > Andrei Barbu wrote:
> > > > > Changelog:
> > > > >
> > > > > Fixes a memory access fault inside of D3D8_GetCreationParameters.
> > > >
> > > > ...
> > > > > HRESULT  WINAPI  
> > > > > IDirect3DDevice8Impl_GetCreationParameters(LPDIRECT3DDEVICE8 iface, 
> > > > > D3DDEVICE_CREATION_PARAMETERS *pParameters) {
> > > > >    ICOM_THIS(IDirect3DDevice8Impl,iface);
> > > > >    TRACE("(%p) copying to %p\n", This, pParameters);    
> > > > > +
> > > > > +    pParameters = (D3DDEVICE_CREATION_PARAMETERS*) malloc( sizeof( 
> > > > > D3DDEVICE_CREATION_PARAMETERS ));
> > > > >    memcpy(pParameters, &This->CreateParms, 
> > > > >    sizeof(D3DDEVICE_CREATION_PARAMETERS));
> > > > >    return D3D_OK;
> > > > > }
> > >
> > > Its also wrong, because the patch just overwrites the pParameters argument
> > > and does not return the allocated memory.
> > >
> > > Ciao, Marcus

D3D8_GetCreationParameters fix

[Andrei Barbu]

Unfortunately I've never actually had to use those before.
I'll look them up. Which one would be best in this case?

On Thu, 2004-07-08 at 15:39, Mike McCormack wrote:
> You probably want to use a Windows method to allocate memory here, such 
> as HeapAlloc() or CoTaskMemAlloc, as the caller is not going to use 
> free() to free the memory...
>
> Mike
>
> Andrei Barbu wrote:
> > Changelog:
> >
> > Fixes a memory access fault inside of D3D8_GetCreationParameters.
> > ...
> >  HRESULT  WINAPI  IDirect3DDevice8Impl_GetCreationParameters(LPDIRECT3DDEVICE8 iface, D3DDEVICE_CREATION_PARAMETERS *pParameters) {
> >      ICOM_THIS(IDirect3DDevice8Impl,iface);
> >      TRACE("(%p) copying to %p\n", This, pParameters);    
> > +
> > +    pParameters = (D3DDEVICE_CREATION_PARAMETERS*) malloc( sizeof( D3DDEVICE_CREATION_PARAMETERS ));
> >      memcpy(pParameters, &This->CreateParms, sizeof(D3DDEVICE_CREATION_PARAMETERS));
> >      return D3D_OK;
> >  }