Re: dmusic: remove unused variable

[Nikolay Sivov]

On 7/21/2010 08:31, Austin English wrote:
> HRESULT WINAPI DMUSIC_CreateDirectMusicPortImpl (LPCGUID lpcGUID, LPVOID *ppobj, LPUNKNOWN pUnkOuter, LPDMUS_PORTPARAMS pPortParams, LPDMUS_PORTCAPS pPortCaps) {
>    IDirectMusicPortImpl *obj;
> -
>
> HRESULT hr = E_FAIL;
> UINT j;
>
> TRACE("(%p,%p,%p)\n", lpcGUID, ppobj, pUnkOuter);
>
> @@ -381,7 +380,6 @@ HRESULT WINAPI DMUSIC_CreateDirectMusicPortImpl (LPCGUID lpcGUID, LPVOID *ppobj,
>    obj->caps = *pPortCaps;
>    obj->pDirectSound = NULL;
>    obj->pLatencyClock = NULL;
> -
>
> hr = DMUSIC_CreateReferenceClockImpl(&IID_IReferenceClock, (LPVOID*)&obj->pLatencyClock, NULL);
>
> if(0)
> {

I doubt you could remove a call here. And probably return value should 
be used to exit earlier on a failure.

dmusic: remove unused variable

[Austin English]

On Wed, Jul 21, 2010 at 12:41 AM, Nikolay Sivov  wrote:
> On 7/21/2010 08:31, Austin English wrote:
> > HRESULT WINAPI DMUSIC_CreateDirectMusicPortImpl (LPCGUID lpcGUID, LPVOID
> > *ppobj, LPUNKNOWN pUnkOuter, LPDMUS_PORTPARAMS pPortParams, LPDMUS_PORTCAPS
> > pPortCaps) {
> >        IDirectMusicPortImpl *obj;
> > - HRESULT hr = E_FAIL;
> >
> > UINT j;
> >
> > TRACE("(%p,%p,%p)\n", lpcGUID, ppobj, pUnkOuter);
> > @@ -381,7 +380,6 @@ HRESULT WINAPI DMUSIC_CreateDirectMusicPortImpl
> > (LPCGUID lpcGUID, LPVOID *ppobj,
> >        obj->caps = *pPortCaps;
> >        obj->pDirectSound = NULL;
> >        obj->pLatencyClock = NULL;
> > - hr = DMUSIC_CreateReferenceClockImpl(&IID_IReferenceClock,
> >
> > (LPVOID*)&obj->pLatencyClock, NULL);
> >
> > if(0)
> >  {
>
> I doubt you could remove a call here. And probably return value should be
> used to exit earlier on a failure.

Hm, good point. How's this?

diff --git a/dlls/dmusic/port.c b/dlls/dmusic/port.c
index 021f1e2..23fa1b8 100644
--- a/dlls/dmusic/port.c
+++ b/dlls/dmusic/port.c
@@ -382,7 +382,8 @@ HRESULT WINAPI DMUSIC_CreateDirectMusicPortImpl
(LPCGUID lpcGUID, LPVOID *ppobj,
        obj->pDirectSound = NULL;
        obj->pLatencyClock = NULL;
        hr = DMUSIC_CreateReferenceClockImpl(&IID_IReferenceClock,
(LPVOID*)&obj->pLatencyClock, NULL);
-
+    if(hr != S_OK)
+        return E_FAIL;
 if(0)
 {
        if (pPortParams->dwValidParams & DMUS_PORTPARAMS_CHANNELGROUPS) {

Thanks for reviewing!

dmusic: remove unused variable

[Nikolay Sivov]

On 7/21/2010 11:16, Austin English wrote:
> On Wed, Jul 21, 2010 at 12:41 AM, Nikolay Sivov  wrote:
> > On 7/21/2010 08:31, Austin English wrote:
> > > HRESULT WINAPI DMUSIC_CreateDirectMusicPortImpl (LPCGUID lpcGUID, LPVOID
> > > *ppobj, LPUNKNOWN pUnkOuter, LPDMUS_PORTPARAMS pPortParams, LPDMUS_PORTCAPS
> > > pPortCaps) {
> > >         IDirectMusicPortImpl *obj;
> > > -
> > >   HRESULT hr = E_FAIL;
> > >   UINT j;
> > >
> > >   TRACE("(%p,%p,%p)\n", lpcGUID, ppobj, pUnkOuter);
> > >
> > > @@ -381,7 +380,6 @@ HRESULT WINAPI DMUSIC_CreateDirectMusicPortImpl
> > > (LPCGUID lpcGUID, LPVOID *ppobj,
> > >         obj->caps = *pPortCaps;
> > >         obj->pDirectSound = NULL;
> > >         obj->pLatencyClock = NULL;
> > > -
> > >   hr = DMUSIC_CreateReferenceClockImpl(&IID_IReferenceClock,
> > >
> > > (LPVOID*)&obj->pLatencyClock, NULL);
> > >
> > > if(0)
> > >   {
> >
> > I doubt you could remove a call here. And probably return value should be
> > used to exit earlier on a failure.
>
> Hm, good point. How's this?
>
> diff --git a/dlls/dmusic/port.c b/dlls/dmusic/port.c
> index 021f1e2..23fa1b8 100644
> --- a/dlls/dmusic/port.c
> +++ b/dlls/dmusic/port.c
> @@ -382,7 +382,8 @@ HRESULT WINAPI DMUSIC_CreateDirectMusicPortImpl
> (LPCGUID lpcGUID, LPVOID *ppobj,
>          obj->pDirectSound = NULL;
>          obj->pLatencyClock = NULL;
>          hr = DMUSIC_CreateReferenceClockImpl(&IID_IReferenceClock,
> (LPVOID*)&obj->pLatencyClock, NULL);
> -
> - if(hr != S_OK)
> -
>    return E_FAIL;
> if(0)
> {
>        if (pPortParams->dwValidParams&  DMUS_PORTPARAMS_CHANNELGROUPS) {
>
> Thanks for reviewing!

Don't know, I'm not familiar with this at all. You need to figure out if 
this failure is critical enough to exit, and also it makes sense to 
return hr as is, not E_FAIL.

dmusic: remove unused variable

[Henri Verbeet]

On 21 July 2010 09:21, Nikolay Sivov  wrote:
> On 7/21/2010 11:16, Austin English wrote:
> > Hm, good point. How's this?
> >
> > diff --git a/dlls/dmusic/port.c b/dlls/dmusic/port.c
> > index 021f1e2..23fa1b8 100644
> > --- a/dlls/dmusic/port.c
> > +++ b/dlls/dmusic/port.c
> > @@ -382,7 +382,8 @@ HRESULT WINAPI DMUSIC_CreateDirectMusicPortImpl
> > (LPCGUID lpcGUID, LPVOID *ppobj,
> >         obj->pDirectSound = NULL;
> >         obj->pLatencyClock = NULL;
> >         hr = DMUSIC_CreateReferenceClockImpl(&IID_IReferenceClock,
> > (LPVOID*)&obj->pLatencyClock, NULL);
> > -
> > - if(hr != S_OK)
> > - return E_FAIL;
> >
> > if(0)
> >  {
> >         if (pPortParams->dwValidParams&  DMUS_PORTPARAMS_CHANNELGROUPS) {
> >
> > Thanks for reviewing!
>
> Don't know, I'm not familiar with this at all. You need to figure out if
> this failure is critical enough to exit, and also it makes sense to return
> hr as is, not E_FAIL.

I don't know what I'm talking about either, but at the very least
you'd be leaking "obj".