sound3d patch

[Martin]

Hi.

I am working on a patch to fix a bug which prevents certain games from 
starting. The bug will make wine spit out:

fixme:dsound3d:IDirectSound3DListenerImpl_QueryInterface Unknown IID {31efac30-515c-11d0-a9aa-00aa0061be93} right before it crashes(see bug 12240).

I am working on implementing the ability to return this interface by calling the function that fails(IDirectSound3DListenerImpl_QueryInterface())
with an already acquired (dsound?) interface and IID being IKsPropertySet.
I was able to make a patch which helped me get around this bug in one game, but I don't have very deep knowledge of how wine works internally. Because of that,
I am not sure if I should implement something more to prevent other issues. Also, I don't know if I need to make some changes to make the patch more "correct" or complete.
If somebody could have a look at the patch, or try it out or give me any other feedback, that would be appreciated.

Thanks for any feedback!
Martin

[Maarten Lankhorst]

Hi Martin,

2008/6/27 Martin :
> Hi.
>
> I am working on a patch to fix a bug which prevents certain games from
> starting. The bug will make wine spit out:
>
> fixme:dsound3d:IDirectSound3DListenerImpl_QueryInterface Unknown IID
> {31efac30-515c-11d0-a9aa-00aa0061be93} right before it crashes(see bug
> 12240).
>
> I am working on implementing the ability to return this interface by calling
> the function that fails(IDirectSound3DListenerImpl_QueryInterface())
> with an already acquired (dsound?) interface and IID being IKsPropertySet.
> I was able to make a patch which helped me get around this bug in one game,
> but I don't have very deep knowledge of how wine works internally. Because
> of that,
> I am not sure if I should implement something more to prevent other issues.
> Also, I don't know if I need to make some changes to make the patch more
> "correct" or complete.
> If somebody could have a look at the patch, or try it out or give me any
> other feedback, that would be appreciated.

If you aren't sure, try creating a simple test for it.

Cheers,
Maarten.

[Chris Ahrendt]

Maarten Lankhorst wrote:
> Hi Martin,
>
> 2008/6/27 Martin :
> > Hi.
> >
> > I am working on a patch to fix a bug which prevents certain games from
> > starting. The bug will make wine spit out:
> >
> > fixme:dsound3d:IDirectSound3DListenerImpl_QueryInterface Unknown IID
> > {31efac30-515c-11d0-a9aa-00aa0061be93} right before it crashes(see bug
> > 12240).
> >
> > I am working on implementing the ability to return this interface by calling
> > the function that fails(IDirectSound3DListenerImpl_QueryInterface())
> > with an already acquired (dsound?) interface and IID being IKsPropertySet.
> > I was able to make a patch which helped me get around this bug in one game,
> > but I don't have very deep knowledge of how wine works internally. Because
> > of that,
> > I am not sure if I should implement something more to prevent other issues.
> > Also, I don't know if I need to make some changes to make the patch more
> > "correct" or complete.
> > If somebody could have a look at the patch, or try it out or give me any
> > other feedback, that would be appreciated.
>
> If you aren't sure, try creating a simple test for it.
>
> Cheers,
> Maarten.

Someone once told me that coding is 10% actual new coding or bug fixes 
and the 90% left was writing test cases and test code for the code .

The fix looks ok to me... but I am also a new guy here too =D

[Chris Ahrendt]

Maarten Lankhorst wrote:
> Hi Martin,
>
> 2008/6/27 Martin :
> > Hi.
> >
> > I am working on a patch to fix a bug which prevents certain games from
> > starting. The bug will make wine spit out:
> >
> > fixme:dsound3d:IDirectSound3DListenerImpl_QueryInterface Unknown IID
> > {31efac30-515c-11d0-a9aa-00aa0061be93} right before it crashes(see bug
> > 12240).
> >
> > I am working on implementing the ability to return this interface by calling
> > the function that fails(IDirectSound3DListenerImpl_QueryInterface())
> > with an already acquired (dsound?) interface and IID being IKsPropertySet.
> > I was able to make a patch which helped me get around this bug in one game,
> > but I don't have very deep knowledge of how wine works internally. Because
> > of that,
> > I am not sure if I should implement something more to prevent other issues.
> > Also, I don't know if I need to make some changes to make the patch more
> > "correct" or complete.
> > If somebody could have a look at the patch, or try it out or give me any
> > other feedback, that would be appreciated.
>
> If you aren't sure, try creating a simple test for it.
>
> Cheers,
> Maarten.

This would probably be a better patch:

int nobuff = This->device->nrofbuffers - 1;
char *buffer = NULL;

if  (nobuff < 0) nobuff = 0;
  else
   if (!nobuff) nobuff = 1;
if  (!( This->device->buffers[no buff] ))
  {
  ERR(" no device\n");
  return false;
  }

buffer = This->device->buffers[no buff];

if (This->device->buffers)
IKsBufferPropertySetImpl_Create(nobuff,(&This->device->buffers[nobuff]->iks));

Chrisb

[Juan Lang]

> If somebody could have a look at the patch, or try it out or give me any
> other feedback, that would be appreciated.

I'm confused:
+                if (!This->device->buffers)
+                        IKsBufferPropertySetImpl_Create(
(This->device->buffers[This->device->nrofbuffers - 1]),
(&This->device->buffers[This->device->nrofbuffers - 1]->iks));
Aren't you dereferencing a NULL pointer here?  You've just ensured
This->device->buffers is NULL, so accessing it is guaranteed to crash.

+                WARN("IID_IKsPropertySet\n");
+                return E_NOINTERFACE;
What does that warning tell us?  I'd prefer you have something a bit
more descriptive.

Also, the original code uses tabs while you're using spaces.  Please
stick with the existing indentation style.
--Juan