[PATCH 1/5] ole32: Handle GetObject() failure in ParseDisplayName() for item moniker.
Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/ole32/itemmoniker.c | 10 ++++++---- dlls/ole32/tests/moniker.c | 4 ---- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/dlls/ole32/itemmoniker.c b/dlls/ole32/itemmoniker.c index 79fb293c38..73ed63fe6d 100644 --- a/dlls/ole32/itemmoniker.c +++ b/dlls/ole32/itemmoniker.c @@ -790,13 +790,15 @@ static HRESULT WINAPI ItemMonikerImpl_ParseDisplayName(IMoniker *iface, IBindCtx { hr = IOleItemContainer_GetObject(container, This->itemName, get_bind_speed_from_bindctx(pbc), pbc, &IID_IParseDisplayName, (void **)&parser); + if (SUCCEEDED(hr)) + { + hr = IMoniker_GetDisplayName(iface,pbc,NULL,&displayName); - hr = IMoniker_GetDisplayName(iface,pbc,NULL,&displayName); - - hr = IParseDisplayName_ParseDisplayName(parser, pbc, displayName, eaten, ppmkOut); + hr = IParseDisplayName_ParseDisplayName(parser, pbc, displayName, eaten, ppmkOut); + IParseDisplayName_Release(parser); + } IOleItemContainer_Release(container); - IParseDisplayName_Release(parser); } return hr; diff --git a/dlls/ole32/tests/moniker.c b/dlls/ole32/tests/moniker.c index bbd964e723..7607468d01 100644 --- a/dlls/ole32/tests/moniker.c +++ b/dlls/ole32/tests/moniker.c @@ -2141,28 +2141,24 @@ todo_wine hr = IBindCtx_SetBindOptions(bindctx, &bind_opts); ok(hr == S_OK, "Failed to set bind options, hr %#x.\n", hr); hr = IMoniker_ParseDisplayName(moniker, bindctx, &container_moniker->IMoniker_iface, displayname, &eaten, &moniker2); -todo_wine ok(hr == (0x8bee0000 | BINDSPEED_INDEFINITE), "Unexpected hr %#x.\n", hr); bind_opts.dwTickCountDeadline = 1; hr = IBindCtx_SetBindOptions(bindctx, &bind_opts); ok(hr == S_OK, "Failed to set bind options, hr %#x.\n", hr); hr = IMoniker_ParseDisplayName(moniker, bindctx, &container_moniker->IMoniker_iface, displayname, &eaten, &moniker2); -todo_wine ok(hr == (0x8bee0000 | BINDSPEED_IMMEDIATE), "Unexpected hr %#x.\n", hr); bind_opts.dwTickCountDeadline = 2499; hr = IBindCtx_SetBindOptions(bindctx, &bind_opts); ok(hr == S_OK, "Failed to set bind options, hr %#x.\n", hr); hr = IMoniker_ParseDisplayName(moniker, bindctx, &container_moniker->IMoniker_iface, displayname, &eaten, &moniker2); -todo_wine ok(hr == (0x8bee0000 | BINDSPEED_IMMEDIATE), "Unexpected hr %#x.\n", hr); bind_opts.dwTickCountDeadline = 2500; hr = IBindCtx_SetBindOptions(bindctx, &bind_opts); ok(hr == S_OK, "Failed to set bind options, hr %#x.\n", hr); hr = IMoniker_ParseDisplayName(moniker, bindctx, &container_moniker->IMoniker_iface, displayname, &eaten, &moniker2); -todo_wine ok(hr == (0x8bee0000 | BINDSPEED_MODERATE), "Unexpected hr %#x.\n", hr); IMoniker_Release(&container_moniker->IMoniker_iface); -- 2.24.1
Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/ole32/itemmoniker.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/dlls/ole32/itemmoniker.c b/dlls/ole32/itemmoniker.c index 73ed63fe6d..0063c323bc 100644 --- a/dlls/ole32/itemmoniker.c +++ b/dlls/ole32/itemmoniker.c @@ -777,7 +777,6 @@ static HRESULT WINAPI ItemMonikerImpl_ParseDisplayName(IMoniker *iface, IBindCtx ItemMonikerImpl *This = impl_from_IMoniker(iface); IOleItemContainer *container; IParseDisplayName *parser; - LPOLESTR displayName; HRESULT hr; TRACE("%p, %p, %p, %s, %p, %p.\n", iface, pbc, pmkToLeft, debugstr_w(displayname), eaten, ppmkOut); @@ -792,9 +791,7 @@ static HRESULT WINAPI ItemMonikerImpl_ParseDisplayName(IMoniker *iface, IBindCtx &IID_IParseDisplayName, (void **)&parser); if (SUCCEEDED(hr)) { - hr = IMoniker_GetDisplayName(iface,pbc,NULL,&displayName); - - hr = IParseDisplayName_ParseDisplayName(parser, pbc, displayName, eaten, ppmkOut); + hr = IParseDisplayName_ParseDisplayName(parser, pbc, displayname, eaten, ppmkOut); IParseDisplayName_Release(parser); } -- 2.24.1
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=63628 Your paranoid android. === debian10 (32 bit Chinese:China report) === ole32: clipboard.c:1325: Test failed: got 800401d0 clipboard.c:1337: Test failed: got 800401d0 clipboard.c:1338: Test failed: got 0 clipboard.c:1343: Test failed: got 800401d0 clipboard.c:1344: Test failed: got 0 clipboard.c:1349: Test failed: got 800401d0 clipboard.c:1354: Test failed: got 80040064 clipboard.c:1355: Test failed: got 0 clipboard.c:1362: Test failed: got 80040064 clipboard.c:1363: Test failed: got 0 clipboard.c:1368: Test failed: got 80040064 clipboard.c:1373: Test failed: got 80040064 clipboard.c:1374: Test failed: got 0 clipboard.c:1379: Test failed: got 80040064 clipboard.c:1380: Test failed: got 0 clipboard.c:1387: Test failed: got 80040064 clipboard.c:1388: Test failed: got 0 clipboard.c:1398: Test failed: got 80040064 clipboard.c:1399: Test failed: got 0 clipboard.c:1404: Test failed: got 80040064 clipboard.c:1405: Test failed: got 0 clipboard.c:1410: Test failed: got 80040064 clipboard.c:1411: Test failed: got 0 clipboard.c:1416: Test failed: got 80040064 clipboard.c:1417: Test failed: got 0 clipboard.c:1424: Test failed: got 80040064 clipboard.c:1454: Test failed: got 80040064
Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/ole32/itemmoniker.c | 92 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 86 insertions(+), 6 deletions(-) diff --git a/dlls/ole32/itemmoniker.c b/dlls/ole32/itemmoniker.c index 0063c323bc..dc7026c4fc 100644 --- a/dlls/ole32/itemmoniker.c +++ b/dlls/ole32/itemmoniker.c @@ -57,6 +57,84 @@ static inline ItemMonikerImpl *impl_from_IROTData(IROTData *iface) return CONTAINING_RECORD(iface, ItemMonikerImpl, IROTData_iface); } +struct container_lock +{ + IUnknown IUnknown_iface; + LONG refcount; + IOleItemContainer *container; +}; + +static struct container_lock *impl_lock_from_IUnknown(IUnknown *iface) +{ + return CONTAINING_RECORD(iface, struct container_lock, IUnknown_iface); +} + +static HRESULT WINAPI container_lock_QueryInterface(IUnknown *iface, REFIID riid, void **obj) +{ + if (IsEqualIID(riid, &IID_IUnknown)) + { + *obj = iface; + IUnknown_AddRef(iface); + return S_OK; + } + + WARN("Unsupported interface %s.\n", debugstr_guid(riid)); + *obj = NULL; + return E_NOINTERFACE; +} + +static ULONG WINAPI container_lock_AddRef(IUnknown *iface) +{ + struct container_lock *lock = impl_lock_from_IUnknown(iface); + return InterlockedIncrement(&lock->refcount); +} + +static ULONG WINAPI container_lock_Release(IUnknown *iface) +{ + struct container_lock *lock = impl_lock_from_IUnknown(iface); + ULONG refcount = InterlockedDecrement(&lock->refcount); + + if (!refcount) + { + IOleItemContainer_LockContainer(lock->container, FALSE); + IOleItemContainer_Release(lock->container); + heap_free(lock); + } + + return refcount; +} + +static const IUnknownVtbl container_lock_vtbl = +{ + container_lock_QueryInterface, + container_lock_AddRef, + container_lock_Release, +}; + +static HRESULT set_container_lock(IOleItemContainer *container, IBindCtx *pbc) +{ + struct container_lock *lock; + HRESULT hr; + + if (!(lock = heap_alloc(sizeof(*lock)))) + return E_OUTOFMEMORY; + + if (FAILED(hr = IOleItemContainer_LockContainer(container, TRUE))) + { + heap_free(lock); + return hr; + } + + lock->IUnknown_iface.lpVtbl = &container_lock_vtbl; + lock->refcount = 1; + lock->container = container; + IOleItemContainer_AddRef(lock->container); + + hr = IBindCtx_RegisterObjectBound(pbc, &lock->IUnknown_iface); + IUnknown_Release(&lock->IUnknown_iface); + return hr; +} + static HRESULT ItemMonikerImpl_Destroy(ItemMonikerImpl* iface); /******************************************************************************* @@ -787,13 +865,15 @@ static HRESULT WINAPI ItemMonikerImpl_ParseDisplayName(IMoniker *iface, IBindCtx hr = IMoniker_BindToObject(pmkToLeft, pbc, NULL, &IID_IOleItemContainer, (void **)&container); if (SUCCEEDED(hr)) { - hr = IOleItemContainer_GetObject(container, This->itemName, get_bind_speed_from_bindctx(pbc), pbc, - &IID_IParseDisplayName, (void **)&parser); - if (SUCCEEDED(hr)) + if (SUCCEEDED(hr = set_container_lock(container, pbc))) { - hr = IParseDisplayName_ParseDisplayName(parser, pbc, displayname, eaten, ppmkOut); - - IParseDisplayName_Release(parser); + hr = IOleItemContainer_GetObject(container, This->itemName, get_bind_speed_from_bindctx(pbc), pbc, + &IID_IParseDisplayName, (void **)&parser); + if (SUCCEEDED(hr)) + { + hr = IParseDisplayName_ParseDisplayName(parser, pbc, displayname, eaten, ppmkOut); + IParseDisplayName_Release(parser); + } } IOleItemContainer_Release(container); } -- 2.24.1
Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/ole32/itemmoniker.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dlls/ole32/itemmoniker.c b/dlls/ole32/itemmoniker.c index dc7026c4fc..46a16300b7 100644 --- a/dlls/ole32/itemmoniker.c +++ b/dlls/ole32/itemmoniker.c @@ -445,6 +445,9 @@ static HRESULT WINAPI ItemMonikerImpl_BindToObject(IMoniker* iface, hr = IMoniker_BindToObject(pmkToLeft, pbc, NULL, &IID_IOleItemContainer, (void **)&container); if (SUCCEEDED(hr)) { + if (FAILED(hr = set_container_lock(container, pbc))) + WARN("Failed to lock container, hr %#x.\n", hr); + hr = IOleItemContainer_GetObject(container, This->itemName, get_bind_speed_from_bindctx(pbc), pbc, riid, ppvResult); IOleItemContainer_Release(container); -- 2.24.1
Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/ole32/itemmoniker.c | 41 +++++++++++++++++--------------------- dlls/ole32/tests/moniker.c | 5 +++-- 2 files changed, 21 insertions(+), 25 deletions(-) diff --git a/dlls/ole32/itemmoniker.c b/dlls/ole32/itemmoniker.c index 46a16300b7..c85e263774 100644 --- a/dlls/ole32/itemmoniker.c +++ b/dlls/ole32/itemmoniker.c @@ -52,6 +52,8 @@ static inline ItemMonikerImpl *impl_from_IMoniker(IMoniker *iface) return CONTAINING_RECORD(iface, ItemMonikerImpl, IMoniker_iface); } +static ItemMonikerImpl *unsafe_impl_from_IMoniker(IMoniker *iface); + static inline ItemMonikerImpl *impl_from_IROTData(IROTData *iface) { return CONTAINING_RECORD(iface, ItemMonikerImpl, IROTData_iface); @@ -601,34 +603,20 @@ static HRESULT WINAPI ItemMonikerImpl_Enum(IMoniker* iface,BOOL fForward, IEnumM /****************************************************************************** * ItemMoniker_IsEqual ******************************************************************************/ -static HRESULT WINAPI ItemMonikerImpl_IsEqual(IMoniker* iface,IMoniker* pmkOtherMoniker) +static HRESULT WINAPI ItemMonikerImpl_IsEqual(IMoniker *iface, IMoniker *other) { + ItemMonikerImpl *moniker = impl_from_IMoniker(iface), *other_moniker; - CLSID clsid; - LPOLESTR dispName1,dispName2; - IBindCtx* bind; - HRESULT res = S_FALSE; - - TRACE("(%p,%p)\n",iface,pmkOtherMoniker); - - if (!pmkOtherMoniker) return S_FALSE; + TRACE("%p, %p.\n", iface, other); + if (!other) + return E_INVALIDARG; - /* check if both are ItemMoniker */ - if(FAILED (IMoniker_GetClassID(pmkOtherMoniker,&clsid))) return S_FALSE; - if(!IsEqualCLSID(&clsid,&CLSID_ItemMoniker)) return S_FALSE; + other_moniker = unsafe_impl_from_IMoniker(other); + if (!other_moniker) + return S_FALSE; - /* check if both displaynames are the same */ - if(SUCCEEDED ((res = CreateBindCtx(0,&bind)))) { - if(SUCCEEDED (IMoniker_GetDisplayName(iface,bind,NULL,&dispName1))) { - if(SUCCEEDED (IMoniker_GetDisplayName(pmkOtherMoniker,bind,NULL,&dispName2))) { - if(wcscmp(dispName1,dispName2)==0) res = S_OK; - CoTaskMemFree(dispName2); - } - CoTaskMemFree(dispName1); - } - } - return res; + return !wcsicmp(moniker->itemName, other_moniker->itemName) ? S_OK : S_FALSE; } /****************************************************************************** @@ -1002,6 +990,13 @@ static const IMonikerVtbl VT_ItemMonikerImpl = ItemMonikerImpl_IsSystemMoniker }; +static ItemMonikerImpl *unsafe_impl_from_IMoniker(IMoniker *iface) +{ + if (iface->lpVtbl != &VT_ItemMonikerImpl) + return NULL; + return CONTAINING_RECORD(iface, ItemMonikerImpl, IMoniker_iface); +} + /********************************************************************************/ /* Virtual function table for the IROTData class. */ static const IROTDataVtbl VT_ROTDataImpl = diff --git a/dlls/ole32/tests/moniker.c b/dlls/ole32/tests/moniker.c index 7607468d01..e0e3020234 100644 --- a/dlls/ole32/tests/moniker.c +++ b/dlls/ole32/tests/moniker.c @@ -2193,6 +2193,9 @@ todo_wine todo_wine ok(hr == E_INVALIDARG, "Unexpected hr %#x.\n", hr); + hr = IMoniker_IsEqual(moniker, NULL); + ok(hr == E_INVALIDARG, "Unexpected hr %#x.\n", hr); + IMoniker_Release(moniker); /* IsEqual */ @@ -2205,11 +2208,9 @@ todo_wine ok(hr == S_OK, "Failed to create moniker, hr %#x.\n", hr); hr = IMoniker_IsEqual(moniker, moniker2); - todo_wine_if(i == 4 || i == 5) ok(hr == isequal_tests[i].hr, "%d: unexpected result %#x.\n", i, hr); hr = IMoniker_IsEqual(moniker2, moniker); - todo_wine_if(i == 4 || i == 5) ok(hr == isequal_tests[i].hr, "%d: unexpected result %#x.\n", i, hr); IMoniker_Release(moniker); -- 2.24.1
participants (2)
-
Marvin -
Nikolay Sivov