Re: [PATCH 2/3] dmloader: Display new ref in AddRef and Release.

[Alistair Leslie-Hughes]

Hi,

On 6/11/2012 7:05 PM, Christian Costa wrote:
> static HRESULT WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_GetObject(LPDIRECTMUSICLOADER8 iface, LPDMUS_OBJECTDESC pDesc, REFIID riid, LPVOID* ppv)
> @@ -880,7 +884,7 @@ HRESULT WINAPI DMUSIC_CreateDirectMusicLoaderImpl (LPCGUID lpcGUID, LPVOID *ppob
>    	return E_OUTOFMEMORY;
>    }
>    obj->IDirectMusicLoader8_iface.lpVtbl = &DirectMusicLoader_Loader_Vtbl;
> - obj->dwRef = 0; /* will be inited with QueryInterface */
> - obj->ref = 0; /* Will be inited with QueryInterface */

Shouldn't obj->ref start at 1 or else it will never be freed.

Best Regards
  Alistair Leslie-Hughes

[PATCH 2/3] dmloader: Display new ref in AddRef and Release.

[Nikolay Sivov]

On 11/6/2012 14:09, Alistair Leslie-Hughes wrote:
> Hi,
>
> On 6/11/2012 7:05 PM, Christian Costa wrote:
> > static HRESULT WINAPI 
> > IDirectMusicLoaderImpl_IDirectMusicLoader_GetObject(LPDIRECTMUSICLOADER8 
> > iface, LPDMUS_OBJECTDESC pDesc, REFIID riid, LPVOID* ppv)
> > @@ -880,7 +884,7 @@ HRESULT WINAPI DMUSIC_CreateDirectMusicLoaderImpl 
> > (LPCGUID lpcGUID, LPVOID *ppob
> >           return E_OUTOFMEMORY;
> >       }
> >       obj->IDirectMusicLoader8_iface.lpVtbl = 
> > &DirectMusicLoader_Loader_Vtbl;
> > - obj->dwRef = 0; /* will be inited with QueryInterface */
> > - obj->ref = 0; /* Will be inited with QueryInterface */
>
> Shouldn't obj->ref start at 1 or else it will never be freed.

It's supposed to be incremented later with QI call, see comment. This 
module is still dusty and COM cleanup is necessary, this is ugly:

---
     IDirectMusicLoader_SetObject ((LPDIRECTMUSICLOADER8)obj, &Desc);
---
     return IDirectMusicLoaderImpl_IDirectMusicLoader_QueryInterface 
((LPDIRECTMUSICLOADER8)obj, lpcGUID, ppobj);
---
> Best Regards
>  Alistair Leslie-Hughes

[PATCH 2/3] dmloader: Display new ref in AddRef and Release.

[Christian Costa]

Thanks for spotting. I will send an additional patch for that or do an
update.

2012/11/6 Nikolay Sivov
> On 11/6/2012 14:09, Alistair Leslie-Hughes wrote:
> > Hi,
> >
> > On 6/11/2012 7:05 PM, Christian Costa wrote:
> >
> > static HRESULT WINAPI IDirectMusicLoaderImpl_**
> > > IDirectMusicLoader_GetObject(**LPDIRECTMUSICLOADER8 iface,
> > > LPDMUS_OBJECTDESC pDesc, REFIID riid, LPVOID* ppv)
> > > @@ -880,7 +884,7 @@ HRESULT WINAPI DMUSIC_**CreateDirectMusicLoaderImpl
> > > (LPCGUID lpcGUID, LPVOID *ppob
> > >           return E_OUTOFMEMORY;
> > >       }
> > >       obj->IDirectMusicLoader8_**iface.lpVtbl =
> > > &DirectMusicLoader_Loader_**Vtbl;
> > > - obj->dwRef = 0; /* will be inited with QueryInterface */
> > > - obj->ref = 0; /* Will be inited with QueryInterface */
> >
> > Shouldn't obj->ref start at 1 or else it will never be freed.
>
> It's supposed to be incremented later with QI call, see comment. This
> module is still dusty and COM cleanup is necessary, this is ugly:
> IDirectMusicLoader_SetObject ((LPDIRECTMUSICLOADER8)obj, &Desc);
> return IDirectMusicLoaderImpl_**IDirectMusicLoader_**QueryInterface
> ((LPDIRECTMUSICLOADER8)obj, lpcGUID, ppobj);
> > Best Regards
> >  Alistair Leslie-Hughes