Re: [PATCH 3/6 v2] dmusic: Implement IDirectMusic8::SetDirectSound()

[Sebastian Lackner]

On 10.05.2017 15:13, Michael Stefaniuc wrote:
> -
> - if (This->dsound)
> -
>    IDirectSound_Release(This->dsound);
> -
> - if (!dsound) {
> -
>    hr = DirectSoundCreate8(NULL, (IDirectSound8 **)&This->dsound, NULL);
> -
>    if (FAILED(hr))
> -
>        return hr;
> -
>    hr = IDirectSound_SetCooperativeLevel(This->dsound, hwnd ? hwnd : GetForegroundWindow(),
> -
>            DSSCL_PRIORITY);
> -
>    if (FAILED(hr))
> -
>        IDirectSound_Release(This->dsound);
> -
>    return hr;
> - }

Shouldn't you unset This->dsound in the error paths (especially the last one)?

[PATCH 3/6 v2] dmusic: Implement IDirectMusic8::SetDirectSound()

[Michael Stefaniuc]

On 05/11/2017 12:44 AM, Sebastian Lackner wrote:
> On 10.05.2017 15:13, Michael Stefaniuc wrote:
> > -
> > - if (This->dsound)
> > -
> >    IDirectSound_Release(This->dsound);
> > -
> > - if (!dsound) {
> > -
> >    hr = DirectSoundCreate8(NULL, (IDirectSound8 **)&This->dsound, NULL);
> > -
> >    if (FAILED(hr))
> > -
> >        return hr;
> > -
> >    hr = IDirectSound_SetCooperativeLevel(This->dsound, hwnd ? hwnd : GetForegroundWindow(),
> > -
> >            DSSCL_PRIORITY);
> > -
> >    if (FAILED(hr))
> > -
> >        IDirectSound_Release(This->dsound);
> > -
> >    return hr;
> > - }
>
> Shouldn't you unset This->dsound in the error paths (especially the last one)?

DirectSoundCreate8() sets This->dsound to NULL on error, so the first
error path is taken care of.

For the second case I'm undecided, I might as well remove the error
handling for IDirectSound_SetCooperativeLevel() as I don't expect that
to fail.

bye
    michael