[PATCH] dmloader: Only use the GUID when looking for cached objects.
--- dlls/dmloader/loader.c | 50 +----------------------------------------- 1 file changed, 1 insertion(+), 49 deletions(-) diff --git a/dlls/dmloader/loader.c b/dlls/dmloader/loader.c index 0da8323e7e5..c4c54412e12 100644 --- a/dlls/dmloader/loader.c +++ b/dlls/dmloader/loader.c @@ -208,55 +208,7 @@ static HRESULT WINAPI IDirectMusicLoaderImpl_GetObject(IDirectMusicLoader8 *ifac TRACE(": not loaded yet\n"); pObjectEntry = pExistingEntry; } - } - else if ((pDesc->dwValidData & (DMUS_OBJ_FILENAME | DMUS_OBJ_FULLPATH)) && - (pExistingEntry->Desc.dwValidData & (DMUS_OBJ_FILENAME | DMUS_OBJ_FULLPATH)) && - !wcsncmp (pDesc->wszFileName, pExistingEntry->Desc.wszFileName, DMUS_MAX_FILENAME)) { - TRACE(": found it by fullpath filename\n"); - if (pExistingEntry->Desc.dwValidData & DMUS_OBJ_LOADED) { - TRACE(": already loaded\n"); - return IDirectMusicObject_QueryInterface (pExistingEntry->pObject, riid, ppv); - } else { - TRACE(": not loaded yet\n"); - pObjectEntry = pExistingEntry; - } - } - else if ((pDesc->dwValidData & (DMUS_OBJ_NAME | DMUS_OBJ_CATEGORY)) && - (pExistingEntry->Desc.dwValidData & (DMUS_OBJ_NAME | DMUS_OBJ_CATEGORY)) && - !wcsncmp (pDesc->wszName, pExistingEntry->Desc.wszName, DMUS_MAX_NAME) && - !wcsncmp (pDesc->wszCategory, pExistingEntry->Desc.wszCategory, DMUS_MAX_CATEGORY)) { - TRACE(": found it by name and category\n"); - if (pExistingEntry->Desc.dwValidData & DMUS_OBJ_LOADED) { - TRACE(": already loaded\n"); - return IDirectMusicObject_QueryInterface (pExistingEntry->pObject, riid, ppv); - } else { - TRACE(": not loaded yet\n"); - pObjectEntry = pExistingEntry; - } - } - else if ((pDesc->dwValidData & DMUS_OBJ_NAME) && - (pExistingEntry->Desc.dwValidData & DMUS_OBJ_NAME) && - !wcsncmp (pDesc->wszName, pExistingEntry->Desc.wszName, DMUS_MAX_NAME)) { - TRACE(": found it by name\n"); - if (pExistingEntry->Desc.dwValidData & DMUS_OBJ_LOADED) { - TRACE(": already loaded\n"); - return IDirectMusicObject_QueryInterface (pExistingEntry->pObject, riid, ppv); - } else { - TRACE(": not loaded yet\n"); - pObjectEntry = pExistingEntry; - } - } - else if ((pDesc->dwValidData & DMUS_OBJ_FILENAME) && - (pExistingEntry->Desc.dwValidData & DMUS_OBJ_FILENAME) && - !wcsncmp (pDesc->wszFileName, pExistingEntry->Desc.wszFileName, DMUS_MAX_FILENAME)) { - TRACE(": found it by filename\n"); - if (pExistingEntry->Desc.dwValidData & DMUS_OBJ_LOADED) { - TRACE(": already loaded\n"); - return IDirectMusicObject_QueryInterface (pExistingEntry->pObject, riid, ppv); - } else { - TRACE(": not loaded yet\n"); - pObjectEntry = pExistingEntry; - } + break; } } -- 2.24.0
Hello Alistair, any reason you remove that code? I do see that code being hit by some apps (Ludwig3, the DirectMusic samples from the DirectX 8.1b SDK). Also we have similar code in, IDirectMusicLoaderImpl_CacheObject() and IDirectMusicLoaderImpl_ReleaseObject(). thanks bye michael On 11/21/19 8:40 AM, Alistair Leslie-Hughes wrote:
--- dlls/dmloader/loader.c | 50 +----------------------------------------- 1 file changed, 1 insertion(+), 49 deletions(-)
diff --git a/dlls/dmloader/loader.c b/dlls/dmloader/loader.c index 0da8323e7e5..c4c54412e12 100644 --- a/dlls/dmloader/loader.c +++ b/dlls/dmloader/loader.c @@ -208,55 +208,7 @@ static HRESULT WINAPI IDirectMusicLoaderImpl_GetObject(IDirectMusicLoader8 *ifac TRACE(": not loaded yet\n"); pObjectEntry = pExistingEntry; } - } - else if ((pDesc->dwValidData & (DMUS_OBJ_FILENAME | DMUS_OBJ_FULLPATH)) && - (pExistingEntry->Desc.dwValidData & (DMUS_OBJ_FILENAME | DMUS_OBJ_FULLPATH)) && - !wcsncmp (pDesc->wszFileName, pExistingEntry->Desc.wszFileName, DMUS_MAX_FILENAME)) { - TRACE(": found it by fullpath filename\n"); - if (pExistingEntry->Desc.dwValidData & DMUS_OBJ_LOADED) { - TRACE(": already loaded\n"); - return IDirectMusicObject_QueryInterface (pExistingEntry->pObject, riid, ppv); - } else { - TRACE(": not loaded yet\n"); - pObjectEntry = pExistingEntry; - } - } - else if ((pDesc->dwValidData & (DMUS_OBJ_NAME | DMUS_OBJ_CATEGORY)) && - (pExistingEntry->Desc.dwValidData & (DMUS_OBJ_NAME | DMUS_OBJ_CATEGORY)) && - !wcsncmp (pDesc->wszName, pExistingEntry->Desc.wszName, DMUS_MAX_NAME) && - !wcsncmp (pDesc->wszCategory, pExistingEntry->Desc.wszCategory, DMUS_MAX_CATEGORY)) { - TRACE(": found it by name and category\n"); - if (pExistingEntry->Desc.dwValidData & DMUS_OBJ_LOADED) { - TRACE(": already loaded\n"); - return IDirectMusicObject_QueryInterface (pExistingEntry->pObject, riid, ppv); - } else { - TRACE(": not loaded yet\n"); - pObjectEntry = pExistingEntry; - } - } - else if ((pDesc->dwValidData & DMUS_OBJ_NAME) && - (pExistingEntry->Desc.dwValidData & DMUS_OBJ_NAME) && - !wcsncmp (pDesc->wszName, pExistingEntry->Desc.wszName, DMUS_MAX_NAME)) { - TRACE(": found it by name\n"); - if (pExistingEntry->Desc.dwValidData & DMUS_OBJ_LOADED) { - TRACE(": already loaded\n"); - return IDirectMusicObject_QueryInterface (pExistingEntry->pObject, riid, ppv); - } else { - TRACE(": not loaded yet\n"); - pObjectEntry = pExistingEntry; - } - } - else if ((pDesc->dwValidData & DMUS_OBJ_FILENAME) && - (pExistingEntry->Desc.dwValidData & DMUS_OBJ_FILENAME) && - !wcsncmp (pDesc->wszFileName, pExistingEntry->Desc.wszFileName, DMUS_MAX_FILENAME)) { - TRACE(": found it by filename\n"); - if (pExistingEntry->Desc.dwValidData & DMUS_OBJ_LOADED) { - TRACE(": already loaded\n"); - return IDirectMusicObject_QueryInterface (pExistingEntry->pObject, riid, ppv); - } else { - TRACE(": not loaded yet\n"); - pObjectEntry = pExistingEntry; - } + break; } }
Hi Micheal, Game LegoLand. This game loads about 40 objects (sgt,sty) into the cache, and when it's looping over the cache it's selecting the wrong object. Simple example, of 3 objects. 1. GUID XX, Name "Trans" category "chord" (filename trans.sgt) 2. GUID YY, Name "Other" category "tempo" 3. GUID ZZ, Name "Trans" category "chord" (filename trans.sty) Using IDirectMusicLoader8_GetObject Processing element 3, searching for an object with GUID = XX, Name = "Trans" and category = "chord". We find the object 1 (correct) We continue searching and find object 3 (wrong that's ourself). Then we attempt to resolve object 3 by calling _GetObject again, causing a stack overflow. Snippet of the log. 0030:trace:dmloader:dump_DMUS_OBJECTDESC - wszFileName = L"WLtran2.sty" 0030:trace:dmloader:IDirectMusicLoaderImpl_GetObject : looking if we have object in the cache or if it can be found via alias 0030:trace:dmloader:IDirectMusicLoaderImpl_GetObject : found it by object GUID 0030:trace:dmloader:IDirectMusicLoaderImpl_GetObject : not loaded yet 0030:trace:dmloader:IDirectMusicLoaderImpl_GetObject : found it by name and category 0030:trace:dmloader:IDirectMusicLoaderImpl_GetObject : not loaded yet The unique identifiers in this structure are GUID, filename. Also the loop has bad logic. These are the same, so the second if will never be hit. else if ((pDesc->dwValidData & (DMUS_OBJ_FILENAME | DMUS_OBJ_FULLPATH)) && ... else if ((pDesc->dwValidData & DMUS_OBJ_FILENAME) && If we want to keep all conditions, then we could do something beflow, so weight is given each flag. (simplify logic). if (pDesc->dwValidData & DMUS_OBJ_OBJECT) { if(IsEqualGUID (&pDesc->guidObject, &pExistingEntry->Desc.guidObject)) pObjectEntry = pExistingEntry; break; } Regards Alistair.
participants (2)
-
Alistair Leslie-Hughes -
Michael Stefaniuc