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

Nikolay Sivov Tue, 06 Nov 2012 10:28:13 -0800

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 andstrip 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 likedirectmusicloader_AddRef() is enough, of course you could use mixedcasing 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,

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

Reply via email to