Re: [PATCH 1/2] dsound: Simplify error handling when creating a sound buffer.

[Sebastian Lackner]

On 22.12.2015 10:49, Michael Stefaniuc wrote:
> Signed-off-by: Michael Stefaniuc <mstefani(a)redhat.de>
> ---
> I run into this and wasted time while fixing the bug during the freeze.
> So I am submitting the dsound cleanups I have laying around.
>
> dlls/dsound/buffer.c | 46 ++++++++++++++++------------------------------
>  1 file changed, 16 insertions(+), 30 deletions(-)

Are you sure this will work as expected? The destructor makes assumptions
about specific variables to be initialized. If I don't miss anything,
DirectSoundDevice_RemoveBuffer could trigger an assertion / corrupt
nrofbuffers now. This->buffer->ref will also not be initialized. Could you
please double check that?

[PATCH 1/2] dsound: Simplify error handling when creating a sound buffer.

[Michael Stefaniuc]

On 12/23/2015 12:57 AM, Sebastian Lackner wrote:
> On 22.12.2015 10:49, Michael Stefaniuc wrote:
> > Signed-off-by: Michael Stefaniuc <mstefani(a)redhat.de>
> > ---
> > I run into this and wasted time while fixing the bug during the freeze.
> > So I am submitting the dsound cleanups I have laying around.
> >
> > dlls/dsound/buffer.c | 46 ++++++++++++++++------------------------------
> >  1 file changed, 16 insertions(+), 30 deletions(-)
>
> Are you sure this will work as expected? The destructor makes assumptions
> about specific variables to be initialized. If I don't miss anything,
> DirectSoundDevice_RemoveBuffer could trigger an assertion / corrupt
> nrofbuffers now. This->buffer->ref will also not be initialized. Could you
> please double check that?
> Pssst... I was just trying to add myself to the wall of fame ;)

Technically you are right. But all those error cases including the
failure of DirectSoundDevice_AddBuffer() are for malloc issues.

The only interesting case from a practical point of view is the failure
to allocate dsb->buffer->memory as the size of that is driven by the
application. Looks like I missed that when writting the patch 2 years
ago. And of course the dsound era applications are all well behaved and
wouldn't ever pass a -1 to a DWORD length... So I need to think how to
deal with that one.

The other allocations are small for sizeof(struct) bytes. If those fail
all hope is lost anyway. Thus handling those failures is basically
useless and that's the reason Alexandre pushes me to restart my work on
the HeapAlloc() wrappers...


bye
        michael