[PATCH 1/3] dmloader: COM cleanup of IDirectMusicLoader object.

Nikolay Sivov bunglehead at gmail.com
Tue Nov 6 13:26:30 CST 2012


On 11/6/2012 20:47, Christian Costa wrote:
> ---
>   dlls/dmloader/dmloader_private.h |   16 +++---
>   dlls/dmloader/loader.c           |  107 +++++++++++++++++++++++---------------
>   2 files changed, 73 insertions(+), 50 deletions(-)
>
> diff --git a/dlls/dmloader/dmloader_private.h b/dlls/dmloader/dmloader_private.h
> index 47a794c..913f3a6 100644
> --- a/dlls/dmloader/dmloader_private.h
> +++ b/dlls/dmloader/dmloader_private.h
> @@ -91,14 +91,14 @@ typedef struct _WINE_LOADER_OPTION {
>    * IDirectMusicLoaderImpl implementation structure
>    */
>   struct IDirectMusicLoaderImpl {
> -	/* VTABLEs */
> -	const IDirectMusicLoader8Vtbl *LoaderVtbl;
> -	/* reference counter */
> -	LONG dwRef;	
> -	/* simple cache (linked list) */
> -	struct list *pObjects;
> -	/* settings for certain object classes */
> -	struct list *pClassSettings;
> +    /* VTABLEs */
> +    IDirectMusicLoader8 IDirectMusicLoader8_iface;
> +    /* reference counter */
> +    LONG dwRef;
> +    /* simple cache (linked list) */
> +    struct list *pObjects;
> +    /* settings for certain object classes */
> +    struct list *pClassSettings;
>   };
Please remove useless comments like VTABLES and reference counter and 
strip names from prefix, e.g. dwRef->ref.
>   
>   /* contained object entry */
> diff --git a/dlls/dmloader/loader.c b/dlls/dmloader/loader.c
> index cfb3de9..54108c7 100644
> --- a/dlls/dmloader/loader.c
> +++ b/dlls/dmloader/loader.c
> @@ -21,6 +21,11 @@
>   
>   WINE_DEFAULT_DEBUG_CHANNEL(dmloader);
>   
> +static inline IDirectMusicLoaderImpl* impl_from_IDirectMusicLoader8(IDirectMusicLoader8 *iface)
> +{
> +    return CONTAINING_RECORD(iface, IDirectMusicLoaderImpl, IDirectMusicLoader8_iface);
> +}
> +
>   static HRESULT DMUSIC_InitLoaderSettings (LPDIRECTMUSICLOADER8 iface);
>   static HRESULT DMUSIC_GetLoaderSettings (LPDIRECTMUSICLOADER8 iface, REFGUID pClassID, WCHAR* wszSearchPath, LPBOOL pbCache);
>   static HRESULT DMUSIC_SetLoaderSettings (LPDIRECTMUSICLOADER8 iface, REFGUID pClassID, WCHAR* wszSearchPath, LPBOOL pbCache);
> @@ -69,8 +74,9 @@ static BOOL DMUSIC_IsValidLoadableClass (REFCLSID pClassID) {
>    */
>   /* IUnknown/IDirectMusicLoader(8) part: */
>   
> -static HRESULT WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_QueryInterface (LPDIRECTMUSICLOADER8 iface, REFIID riid, LPVOID *ppobj) {
> -	ICOM_THIS_MULTI(IDirectMusicLoaderImpl, LoaderVtbl, iface);
> +static HRESULT WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_QueryInterface(LPDIRECTMUSICLOADER8 iface, REFIID riid, LPVOID *ppobj)
> +{
> +	IDirectMusicLoaderImpl *This = impl_from_IDirectMusicLoader8(iface);
>   
>   	TRACE("(%p, %s, %p)\n",This, debugstr_dmguid(riid), ppobj);
>   	if (IsEqualIID (riid, &IID_IUnknown) ||
> @@ -85,15 +91,17 @@ static HRESULT WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_QueryInterface (
>   	return E_NOINTERFACE;
>   }
>   
> -static ULONG WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_AddRef (LPDIRECTMUSICLOADER8 iface) {
> -	ICOM_THIS_MULTI(IDirectMusicLoaderImpl, LoaderVtbl, iface);
> +static ULONG WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_AddRef(LPDIRECTMUSICLOADER8 iface)
> +{
> +	IDirectMusicLoaderImpl *This = impl_from_IDirectMusicLoader8(iface);
>   	TRACE("(%p): AddRef from %d\n", This, This->dwRef);
>   	return InterlockedIncrement (&This->dwRef);
>   }
What I also meant is these names are redundant, something like 
directmusicloader_AddRef() is enough, of course you could use mixed 
casing in names if you want, there's no strict guidelines for that afaik.
>   
> -static ULONG WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_Release (LPDIRECTMUSICLOADER8 iface) {
> -	ICOM_THIS_MULTI(IDirectMusicLoaderImpl, LoaderVtbl, iface);
> -	
> +static ULONG WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_Release(LPDIRECTMUSICLOADER8 iface)
> +{
> +	IDirectMusicLoaderImpl *This = impl_from_IDirectMusicLoader8(iface);
> +
>   	DWORD dwRef = InterlockedDecrement (&This->dwRef);
>   	TRACE("(%p): ReleaseRef to %d\n", This, This->dwRef);
>   	if (dwRef == 0) {
> @@ -107,8 +115,9 @@ static ULONG WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_Release (LPDIRECTM
>   	return dwRef;
>   }
>   
> -static HRESULT WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_GetObject (LPDIRECTMUSICLOADER8 iface, LPDMUS_OBJECTDESC pDesc, REFIID riid, LPVOID* ppv) {
> -	ICOM_THIS_MULTI(IDirectMusicLoaderImpl, LoaderVtbl, iface);
> +static HRESULT WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_GetObject(LPDIRECTMUSICLOADER8 iface, LPDMUS_OBJECTDESC pDesc, REFIID riid, LPVOID* ppv)
> +{
> +	IDirectMusicLoaderImpl *This = impl_from_IDirectMusicLoader8(iface);
>   	HRESULT result = S_OK;
>   	HRESULT ret = S_OK; /* used at the end of function, to determine whether everything went OK */
>   	
> @@ -361,8 +370,9 @@ static HRESULT WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_GetObject (LPDIR
>   		return result;
>   }
>   
> -static HRESULT WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_SetObject (LPDIRECTMUSICLOADER8 iface, LPDMUS_OBJECTDESC pDesc) {
> -	ICOM_THIS_MULTI(IDirectMusicLoaderImpl, LoaderVtbl, iface);
> +static HRESULT WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_SetObject(LPDIRECTMUSICLOADER8 iface, LPDMUS_OBJECTDESC pDesc)
> +{
> +	IDirectMusicLoaderImpl *This = impl_from_IDirectMusicLoader8(iface);
>   	LPSTREAM pStream;
>   	LPDIRECTMUSICOBJECT pObject;
>   	DMUS_OBJECTDESC Desc;
> @@ -483,9 +493,10 @@ static HRESULT WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_SetObject (LPDIR
>   	return S_OK;
>   }
>   
> -static HRESULT WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_SetSearchDirectory (LPDIRECTMUSICLOADER8 iface, REFGUID rguidClass, WCHAR* pwzPath, BOOL fClear) {
> +static HRESULT WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_SetSearchDirectory(LPDIRECTMUSICLOADER8 iface, REFGUID rguidClass, WCHAR* pwzPath, BOOL fClear)
> +{
> +	IDirectMusicLoaderImpl *This = impl_from_IDirectMusicLoader8(iface);
>   	WCHAR wszCurrentPath[MAX_PATH];
> -	ICOM_THIS_MULTI(IDirectMusicLoaderImpl, LoaderVtbl, iface);
>   	TRACE("(%p, %s, %s, %d)\n", This, debugstr_dmguid(rguidClass), debugstr_w(pwzPath), fClear);
>   	FIXME(": fClear ignored\n");
>   	DMUSIC_GetLoaderSettings (iface, rguidClass, wszCurrentPath, NULL);
> @@ -496,14 +507,15 @@ static HRESULT WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_SetSearchDirecto
>   	return DMUSIC_SetLoaderSettings (iface, rguidClass, pwzPath, NULL);
>   }
>   
> -static HRESULT WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_ScanDirectory (LPDIRECTMUSICLOADER8 iface, REFGUID rguidClass, WCHAR* pwzFileExtension, WCHAR* pwzScanFileName) {
> +static HRESULT WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_ScanDirectory(LPDIRECTMUSICLOADER8 iface, REFGUID rguidClass, WCHAR* pwzFileExtension, WCHAR* pwzScanFileName)
> +{
> +	IDirectMusicLoaderImpl *This = impl_from_IDirectMusicLoader8(iface);
>   	static const WCHAR wszAny[] = {'*',0};
>   	WIN32_FIND_DATAW FileData;
>   	HANDLE hSearch;
>   	WCHAR wszSearchString[MAX_PATH];
>   	WCHAR *p;
>   	HRESULT result;
> -	ICOM_THIS_MULTI(IDirectMusicLoaderImpl, LoaderVtbl, iface);
>   	TRACE("(%p, %s, %p, %p)\n", This, debugstr_dmguid(rguidClass), pwzFileExtension, pwzScanFileName);
>   	if (IsEqualGUID (rguidClass, &GUID_DirectMusicAllTypes) || !DMUSIC_IsValidLoadableClass(rguidClass)) {
>   		ERR(": rguidClass invalid CLSID\n");
> @@ -550,13 +562,14 @@ static HRESULT WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_ScanDirectory (L
>   	} while (1);
>   }
>   
> -static HRESULT WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_CacheObject (LPDIRECTMUSICLOADER8 iface, IDirectMusicObject* pObject) {
> +static HRESULT WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_CacheObject(LPDIRECTMUSICLOADER8 iface, IDirectMusicObject* pObject)
> +{
> +	IDirectMusicLoaderImpl *This = impl_from_IDirectMusicLoader8(iface);
>   	DMUS_OBJECTDESC Desc;
>   	HRESULT result = DMUS_E_LOADER_OBJECTNOTFOUND;
>   	struct list *pEntry;
>   	LPWINE_LOADER_ENTRY  pObjectEntry = NULL;
>   
> -	ICOM_THIS_MULTI(IDirectMusicLoaderImpl, LoaderVtbl, iface);
>   	TRACE("(%p, %p)\n", This, pObject);
>   	
>   	/* get descriptor */
> @@ -630,13 +643,14 @@ static HRESULT WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_CacheObject (LPD
>   	return result;
>   }
>   
> -static HRESULT WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_ReleaseObject (LPDIRECTMUSICLOADER8 iface, IDirectMusicObject* pObject) {
> +static HRESULT WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_ReleaseObject(LPDIRECTMUSICLOADER8 iface, IDirectMusicObject* pObject)
> +{
> +	IDirectMusicLoaderImpl *This = impl_from_IDirectMusicLoader8(iface);
>   	DMUS_OBJECTDESC Desc;
>   	struct list *pEntry;
>   	LPWINE_LOADER_ENTRY pObjectEntry = NULL;
>   	HRESULT result = S_FALSE;
>   
> -	ICOM_THIS_MULTI(IDirectMusicLoaderImpl, LoaderVtbl, iface);
>   	TRACE("(%p, %p)\n", This, pObject);
>   	
>   	if(!pObject) return E_POINTER;
> @@ -695,10 +709,11 @@ static HRESULT WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_ReleaseObject (L
>   	return result;
>   }
>   
> -static HRESULT WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_ClearCache (LPDIRECTMUSICLOADER8 iface, REFGUID rguidClass) {
> +static HRESULT WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_ClearCache(LPDIRECTMUSICLOADER8 iface, REFGUID rguidClass)
> +{
> +	IDirectMusicLoaderImpl *This = impl_from_IDirectMusicLoader8(iface);
>   	struct list *pEntry;
>   	LPWINE_LOADER_ENTRY pObjectEntry;
> -	ICOM_THIS_MULTI(IDirectMusicLoaderImpl, LoaderVtbl, iface);
>   	TRACE("(%p, %s)\n", This, debugstr_dmguid(rguidClass));
>   	
>   	LIST_FOR_EACH (pEntry, This->pObjects) {
> @@ -714,8 +729,9 @@ static HRESULT WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_ClearCache (LPDI
>   	return S_OK;
>   }
>   
> -static HRESULT WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_EnableCache (LPDIRECTMUSICLOADER8 iface, REFGUID rguidClass, BOOL fEnable) {
> -	ICOM_THIS_MULTI(IDirectMusicLoaderImpl, LoaderVtbl, iface);
> +static HRESULT WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_EnableCache(LPDIRECTMUSICLOADER8 iface, REFGUID rguidClass, BOOL fEnable)
> +{
> +	IDirectMusicLoaderImpl *This = impl_from_IDirectMusicLoader8(iface);
>   	BOOL bCurrent;
>   	TRACE("(%p, %s, %d)\n", This, debugstr_dmguid(rguidClass), fEnable);
>   	DMUSIC_GetLoaderSettings (iface, rguidClass, NULL, &bCurrent);
> @@ -725,11 +741,12 @@ static HRESULT WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_EnableCache (LPD
>   		return DMUSIC_SetLoaderSettings (iface, rguidClass, NULL, &fEnable);
>   }
>   
> -static HRESULT WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_EnumObject (LPDIRECTMUSICLOADER8 iface, REFGUID rguidClass, DWORD dwIndex, LPDMUS_OBJECTDESC pDesc) {
> +static HRESULT WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_EnumObject(LPDIRECTMUSICLOADER8 iface, REFGUID rguidClass, DWORD dwIndex, LPDMUS_OBJECTDESC pDesc)
> +{
> +	IDirectMusicLoaderImpl *This = impl_from_IDirectMusicLoader8(iface);
>   	DWORD dwCount = 0;
>   	struct list *pEntry;
>   	LPWINE_LOADER_ENTRY pObjectEntry;
> -	ICOM_THIS_MULTI(IDirectMusicLoaderImpl, LoaderVtbl, iface);
>   	TRACE("(%p, %s, %d, %p)\n", This, debugstr_dmguid(rguidClass), dwIndex, pDesc);
>   
>   	DM_STRUCT_INIT(pDesc);
> @@ -755,13 +772,15 @@ static HRESULT WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_EnumObject (LPDI
>   	return S_FALSE;
>   }
>   
> -static void WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_CollectGarbage (LPDIRECTMUSICLOADER8 iface) {
> -	ICOM_THIS_MULTI(IDirectMusicLoaderImpl, LoaderVtbl, iface);
> +static void WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_CollectGarbage(LPDIRECTMUSICLOADER8 iface)
> +{
> +	IDirectMusicLoaderImpl *This = impl_from_IDirectMusicLoader8(iface);
>   	FIXME("(%p): stub\n", This);
>   }
>   
> -static HRESULT WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_ReleaseObjectByUnknown (LPDIRECTMUSICLOADER8 iface, IUnknown* pObject) {
> -	ICOM_THIS_MULTI(IDirectMusicLoaderImpl, LoaderVtbl, iface);
> +static HRESULT WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_ReleaseObjectByUnknown(LPDIRECTMUSICLOADER8 iface, IUnknown* pObject)
> +{
> +	IDirectMusicLoaderImpl *This = impl_from_IDirectMusicLoader8(iface);
>   	HRESULT result;
>   	LPDIRECTMUSICOBJECT pObjectInterface;
>   	
> @@ -781,8 +800,9 @@ static HRESULT WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_ReleaseObjectByU
>   	return result;
>   }
>   
> -static HRESULT WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_LoadObjectFromFile (LPDIRECTMUSICLOADER8 iface, REFGUID rguidClassID, REFIID iidInterfaceID, WCHAR* pwzFilePath, void** ppObject) {
> -	ICOM_THIS_MULTI(IDirectMusicLoaderImpl, LoaderVtbl, iface);
> +static HRESULT WINAPI IDirectMusicLoaderImpl_IDirectMusicLoader_LoadObjectFromFile(LPDIRECTMUSICLOADER8 iface, REFGUID rguidClassID, REFIID iidInterfaceID, WCHAR* pwzFilePath, void** ppObject)
> +{
> +	IDirectMusicLoaderImpl *This = impl_from_IDirectMusicLoader8(iface);
>   	DMUS_OBJECTDESC ObjDesc;
>   	WCHAR wszLoaderSearchPath[MAX_PATH];
>   
> @@ -859,7 +879,7 @@ HRESULT WINAPI DMUSIC_CreateDirectMusicLoaderImpl (LPCGUID lpcGUID, LPVOID *ppob
>   		*ppobj = NULL;
>   		return E_OUTOFMEMORY;
>   	}
> -	obj->LoaderVtbl = &DirectMusicLoader_Loader_Vtbl;
> +	obj->IDirectMusicLoader8_iface.lpVtbl = &DirectMusicLoader_Loader_Vtbl;
>   	obj->dwRef = 0; /* will be inited with QueryInterface */
>   	/* init cache/alias list */
>   	obj->pObjects = HeapAlloc (GetProcessHeap (), HEAP_ZERO_MEMORY, sizeof(struct list));
> @@ -867,7 +887,7 @@ HRESULT WINAPI DMUSIC_CreateDirectMusicLoaderImpl (LPCGUID lpcGUID, LPVOID *ppob
>   	/* init settings */
>   	obj->pClassSettings = HeapAlloc (GetProcessHeap (), HEAP_ZERO_MEMORY, sizeof(struct list));
>   	list_init (obj->pClassSettings);
> -	DMUSIC_InitLoaderSettings ((LPDIRECTMUSICLOADER8)obj);
> +	DMUSIC_InitLoaderSettings(&obj->IDirectMusicLoader8_iface);
>   
>   	/* set default DLS collection (via SetObject... so that loading via DMUS_OBJ_OBJECT is possible) */
>   	DM_STRUCT_INIT(&Desc);
> @@ -875,7 +895,7 @@ HRESULT WINAPI DMUSIC_CreateDirectMusicLoaderImpl (LPCGUID lpcGUID, LPVOID *ppob
>   	Desc.guidClass = CLSID_DirectMusicCollection;
>   	Desc.guidObject = GUID_DefaultGMCollection;
>   	DMUSIC_GetDefaultGMPath (Desc.wszFileName);
> -	IDirectMusicLoader_SetObject ((LPDIRECTMUSICLOADER8)obj, &Desc);
> +	IDirectMusicLoader_SetObject(&obj->IDirectMusicLoader8_iface, &Desc);
>   	/* and now the workaroundTM for "invalid" default DLS; basically,
>   	   my tests showed that if GUID chunk is present in default DLS
>   	   collection, loader treats it as "invalid" and returns
> @@ -889,12 +909,13 @@ HRESULT WINAPI DMUSIC_CreateDirectMusicLoaderImpl (LPCGUID lpcGUID, LPVOID *ppob
>   
>           lock_module();
>   
> -	return IDirectMusicLoaderImpl_IDirectMusicLoader_QueryInterface ((LPDIRECTMUSICLOADER8)obj, lpcGUID, ppobj);
> +	return IDirectMusicLoader_QueryInterface(&obj->IDirectMusicLoader8_iface, lpcGUID, ppobj);
>   }
>   
>   /* help function for retrieval of search path and caching option for certain class */
> -static HRESULT DMUSIC_GetLoaderSettings (LPDIRECTMUSICLOADER8 iface, REFGUID pClassID, WCHAR* wszSearchPath, LPBOOL pbCache) {
> -	ICOM_THIS_MULTI(IDirectMusicLoaderImpl, LoaderVtbl, iface);
> +static HRESULT DMUSIC_GetLoaderSettings(LPDIRECTMUSICLOADER8 iface, REFGUID pClassID, WCHAR* wszSearchPath, LPBOOL pbCache)
> +{
> +	IDirectMusicLoaderImpl *This = impl_from_IDirectMusicLoader8(iface);
>   	struct list *pEntry;
>   	TRACE(": (%p, %s, %p, %p)\n", This, debugstr_dmguid(pClassID), wszSearchPath, pbCache);
>   	
> @@ -912,8 +933,9 @@ static HRESULT DMUSIC_GetLoaderSettings (LPDIRECTMUSICLOADER8 iface, REFGUID pCl
>   }
>   
>   /* help function for setting search path and caching option for certain class */
> -static HRESULT DMUSIC_SetLoaderSettings (LPDIRECTMUSICLOADER8 iface, REFGUID pClassID, WCHAR* wszSearchPath, LPBOOL pbCache) {
> -	ICOM_THIS_MULTI(IDirectMusicLoaderImpl, LoaderVtbl, iface);
> +static HRESULT DMUSIC_SetLoaderSettings(LPDIRECTMUSICLOADER8 iface, REFGUID pClassID, WCHAR* wszSearchPath, LPBOOL pbCache)
> +{
> +	IDirectMusicLoaderImpl *This = impl_from_IDirectMusicLoader8(iface);
>   	struct list *pEntry;
>   	HRESULT result = S_FALSE; /* in case pClassID != GUID_DirectMusicAllTypes and not a valid CLSID */
>   	TRACE(": (%p, %s, %p, %p)\n", This, debugstr_dmguid(pClassID), wszSearchPath, pbCache);
> @@ -935,9 +957,10 @@ static HRESULT DMUSIC_SetLoaderSettings (LPDIRECTMUSICLOADER8 iface, REFGUID pCl
>   	return result;
>   }
>   
> -static HRESULT DMUSIC_InitLoaderSettings (LPDIRECTMUSICLOADER8 iface) {
> -	ICOM_THIS_MULTI(IDirectMusicLoaderImpl, LoaderVtbl, iface);
> -	
> +static HRESULT DMUSIC_InitLoaderSettings(LPDIRECTMUSICLOADER8 iface)
> +{
> +	IDirectMusicLoaderImpl *This = impl_from_IDirectMusicLoader8(iface);
> +
>   	/* hard-coded list of classes */
>   	static REFCLSID classes[] = {
>   		&CLSID_DirectMusicAudioPathConfig,
>
>
>
>




More information about the wine-devel mailing list