Re: [PATCH] ole32: CreateDataCache should return a COleCache object by default.

[Huw Davies]

On Tue, Sep 26, 2017 at 01:00:08AM +0000, Alistair Leslie-Hughes wrote:
> Signed-off-by: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com>
> ---
>  dlls/ole32/datacache.c  |  5 ++++-
>  dlls/ole32/tests/ole2.c | 10 ++++++++--
>  2 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/dlls/ole32/datacache.c b/dlls/ole32/datacache.c
> index a95d0fb..c563118 100644
> --- a/dlls/ole32/datacache.c
> +++ b/dlls/ole32/datacache.c
> @@ -2760,7 +2760,10 @@ HRESULT WINAPI CreateDataCache(
>    if (newCache == 0)
>      return E_OUTOFMEMORY;
>
> -  hr = IUnknown_QueryInterface(&newCache->IUnknown_inner, riid, ppvObj);
> +  if ( !pUnkOuter && IsEqualIID(&IID_IUnknown, riid) )
> +      hr = IUnknown_QueryInterface(&newCache->IUnknown_inner, &IID_IOleCache2, ppvObj);
> +  else
> +      hr = IUnknown_QueryInterface(&newCache->IUnknown_inner, riid, ppvObj);
>    IUnknown_Release(&newCache->IUnknown_inner);

But now if you call QI(IID_IUnknown) on this interface you'll get
back something different, which is wrong.  You'll need to do
some more restructing to make this correct.

Btw, is there a bug that depends on this?
> diff --git a/dlls/ole32/tests/ole2.c b/dlls/ole32/tests/ole2.c
> index b30f63b..39b161c 100644
> --- a/dlls/ole32/tests/ole2.c
> +++ b/dlls/ole32/tests/ole2.c
> @@ -1575,6 +1575,7 @@ static void test_data_cache(void)
>      char szSystemDir[MAX_PATH];
>      WCHAR wszPath[MAX_PATH];
>      static const WCHAR wszShell32[] = {'\\','s','h','e','l','l','3','2','.','d','l','l',0};
> +    DWORD connection;
>
> static const struct expected_method methods_cacheinitnew[] =
>      {
> @@ -1651,14 +1652,19 @@ static void test_data_cache(void)
>      ok(hr == S_OK, "got 0x%08x\n", hr);
>      hr = IUnknown_QueryInterface(unk, &IID_IOleCache2, (void**)&pOleCache);
>      ok(hr == S_OK, "got 0x%08x\n", hr);
> -todo_wine {
>      ok(unk == (IUnknown*)olecache, "got %p, expected %p\n", olecache, unk);
>      ok(unk == (IUnknown*)pOleCache, "got %p, expected %p\n", pOleCache, unk);
> -}
> +
>      IOleCache2_Release(pOleCache);
>      IOleCache_Release(olecache);
>      IUnknown_Release(unk);
>
> +    hr = CreateDataCache(NULL, &CLSID_NULL, &IID_IUnknown, (void**)&olecache);
> +    ok(hr == S_OK, "got 0x%08x\n", hr);
> +    hr = IOleCache_Cache(olecache, &fmtetc, ADVF_PRIMEFIRST | ADVFCACHE_NOHANDLER, &connection);
> +    ok(hr == S_OK, "got 0x%08x\n", hr);
> +    IOleCache_Release(olecache);
> +

A better test would be to call QI(IID_IOleCache) on the returned
IUnknown* and show that it's the same.  You could also
QI(IID_IUnknown) to test for my comment above.

Huw.

[PATCH] ole32: CreateDataCache should return a COleCache object by default.

[Alistair Leslie-Hughes]

Hi Huw,

Thanks of the review.
> > +      hr = IUnknown_QueryInterface(&newCache->IUnknown_inner, riid, ppvObj);
> >     IUnknown_Release(&newCache->IUnknown_inner);
> > But now if you call QI(IID_IUnknown) on this interface you'll get
> > back something different, which is wrong.  You'll need to do
> > some more restructing to make this correct.
> Btw, is there a bug that depends on this?

https://bugs.winehq.org/show_bug.cgi?id=7369
> A better test would be to call QI(IID_IOleCache) on the returned
> IUnknown* and show that it's the same.  You could also
> QI(IID_IUnknown) to test for my comment above.

I'll have another go at it.

Alistair.