Re: [PATCH v3] ole32: Implement CreateObjrefMoniker().

[Nikolay Sivov]

On 10/13/21 4:31 PM, Dmitry Timoshkov wrote:
> v3: Use separate implementation from pointer moniker.
> Signed-off-by: Dmitry Timoshkov
>
> dlls/ole32/moniker.h        |   4 +-
>  dlls/ole32/ole32.spec       |   2 +-
>  dlls/ole32/ole32_objidl.idl |   6 +
>  dlls/ole32/oleproxy.c       |  13 +
>  dlls/ole32/pointermoniker.c | 531 +++++++++++++++++++++++++++++++++++-
>  dlls/ole32/tests/moniker.c  | 221 ++++++++++++++-
>  include/objidl.idl          |   5 +-
>  7 files changed, 776 insertions(+), 6 deletions(-)

Does not have to be in the same file either. This will avoid multiple
*_impl_from_* helpers.
> -
>
> +static HRESULT WINAPI ObjrefMonikerImpl_Load(IMoniker *iface, IStream *stream)
> +{
> - TRACE("(%p)\n", stream);
> -
> - return E_NOTIMPL;
>
> +}
> -
>
> +static HRESULT WINAPI ObjrefMonikerImpl_Save(IMoniker *iface, IStream *stream, BOOL dirty)
> +{
> - FIXME("(%p,%p,%d): stub\n", iface, stream, dirty);
> - return E_NOTIMPL;
>
> +}

Save() definitely works for objref moniker, so surely Load() is a stub too.
> -
>
> +static HRESULT WINAPI ObjrefMonikerImpl_GetSizeMax(IMoniker* iface, ULARGE_INTEGER *size)
> +{
> - TRACE("(%p,%p)\n", iface, size);
> -
> - if (!size)
> -
>    return E_POINTER;
> -
> - size->u.LowPart = 0;
> - size->u.HighPart = 0;
> -
> - return E_NOTIMPL;
>
> +}

This is incorrect.
> -
>
> +static HRESULT WINAPI ObjrefMonikerImpl_BindToObject(IMoniker *iface, IBindCtx *pbc, IMoniker *left,
> -
>    REFIID riid, void **result)
>
> +{
> - ObjrefMonikerImpl *moniker = objref_impl_from_IMoniker(iface);
> -
> - TRACE("(%p,%p,%p,%s,%p)\n", iface, pbc, left, debugstr_guid(riid), result);
> -
> - if (!moniker->pObject)
> -
>    return E_UNEXPECTED;
> -
> - return IUnknown_QueryInterface(moniker->pObject, riid, result);
>
> +}
> -
>
> +static HRESULT WINAPI ObjrefMonikerImpl_BindToStorage(IMoniker *iface, IBindCtx *pbc, IMoniker *left,
> -
>    REFIID riid, void **result)
>
> +{
> - ObjrefMonikerImpl *moniker = objref_impl_from_IMoniker(iface);
> -
> - TRACE("(%p,%p,%p,%s,%p)\n", iface, pbc, left, debugstr_guid(riid), result);
> -
> - if (!moniker->pObject)
> -
>    return E_UNEXPECTED;
> -
> - return IUnknown_QueryInterface(moniker->pObject, riid, result);
>
> +}
> -

I suspect this is a difference how moniker was created. If it was loaded
from stream, you might not have a usable pObject, only referencing string.

This has to be tested.
> +}
> -
>
> +static HRESULT WINAPI ObjrefMonikerImpl_IsRunning(IMoniker *iface, IBindCtx *pbc, IMoniker *left,
> -
>    IMoniker *running)
>
> +{
> - TRACE("(%p,%p,%p,%p)\n", iface, pbc, left, running);
> -
> - return S_OK;
>
> +}

Same here, see documentation page for it.
> -
>
> +static HRESULT WINAPI ObjrefMonikerImpl_GetTimeOfLastChange(IMoniker *iface,
> -
>                           IBindCtx *pbc, IMoniker *left, FILETIME *time)
>
> +{
> - TRACE("(%p,%p,%p,%p)\n", iface, pbc, left, time);
> - return MK_E_UNAVAILABLE;
>
> +}
> -
>
> +static HRESULT WINAPI ObjrefMonikerImpl_Inverse(IMoniker *iface, IMoniker **moniker)
> +{
> - TRACE("(%p,%p)\n", iface, moniker);
> -
> - return CreateAntiMoniker(moniker);
>
> +}
> -
>
> +static HRESULT WINAPI ObjrefMonikerImpl_CommonPrefixWith(IMoniker *iface, IMoniker *other, IMoniker **prefix)
> +{
> - TRACE("(%p,%p,%p)\n", iface, other, prefix);
> -
> - if (!prefix || !other)
> -
>    return E_INVALIDARG;
> -
> - *prefix = NULL;
> -
> - if (ObjrefMonikerImpl_IsEqual(iface, other) == S_OK)
> - {
> -
>    IMoniker_AddRef(iface);
> -
> -
>    *prefix = iface;
> -
> -
>    return MK_S_US;
> - }
> - else
> -
>    return MK_E_NOPREFIX;
>
> +}

This one is documented differently, if you don't need it, just leave a stub.
> -
>
> +static HRESULT WINAPI ObjrefMonikerImpl_ParseDisplayName(IMoniker *iface, IBindCtx *pbc,
> -
>    IMoniker *left, LPOLESTR name, ULONG *eaten, IMoniker **out)
>
> +{
> - ObjrefMonikerImpl *moniker = objref_impl_from_IMoniker(iface);
> - HRESULT hr;
> - IParseDisplayName *pdn;
> -
> - TRACE("(%p,%p,%p,%p,%p,%p)\n", iface, pbc, left, name, eaten, out);
> -
> - if (left)
> -
>    return MK_E_SYNTAX;
> -
> - if (!moniker->pObject)
> -
>    return E_UNEXPECTED;
> -
> - hr = IUnknown_QueryInterface(moniker->pObject, &IID_IParseDisplayName, (void **)&pdn);
> - if (FAILED(hr))
> -
>    return hr;
> -
> - hr = IParseDisplayName_ParseDisplayName(pdn, pbc, name, eaten, out);
> - IParseDisplayName_Release(pdn);
> -
> - return hr;
>
> +}

Any evidence of it actually doing this? In general, let's leave more
stubs than copies of pointer moniker methods, unless it's clearly a
right thing to do. For BindTo* methods that I suspect you actually need,
calling then semi-stub is better.