On 10/13/21 3:13 PM, Dmitry Timoshkov wrote:
Nikolay Sivov nsivov@codeweavers.com wrote:
+static const IMonikerVtbl VT_ObjrefMonikerImpl = +{
- ObjrefMonikerImpl_QueryInterface,
- PointerMonikerImpl_AddRef,
- PointerMonikerImpl_Release,
- ObjrefMonikerImpl_GetClassID,
- PointerMonikerImpl_IsDirty,
- PointerMonikerImpl_Load,
- ObjrefMonikerImpl_Save,
- PointerMonikerImpl_GetSizeMax,
- PointerMonikerImpl_BindToObject,
- PointerMonikerImpl_BindToStorage,
- PointerMonikerImpl_Reduce,
- PointerMonikerImpl_ComposeWith,
- ObjrefMonikerImpl_Enum,
- PointerMonikerImpl_IsEqual,
- PointerMonikerImpl_Hash,
- PointerMonikerImpl_IsRunning,
- ObjrefMonikerImpl_GetTimeOfLastChange,
- PointerMonikerImpl_Inverse,
- PointerMonikerImpl_CommonPrefixWith,
- PointerMonikerImpl_RelativePathTo,
- ObjrefMonikerImpl_GetDisplayName,
- PointerMonikerImpl_ParseDisplayName,
- ObjrefMonikerImpl_IsSystemMoniker
+};
I think separate implementation would be better. According to docs more methods are supposed to differ, like running state handling, or Load()/GetSizeMax().
Initially I created a fully separate implementation, however after looking at the test results I moved to a less intrusive version. I think that once a method that differs in behaviour is found it's fairly easy to add another standalone implementation.
You already found that it differs, and at least methods mentioned above will differ as well.
With reused functions traces will be misleading as well.
It's worth checking if objref moniker keeps a reference to passed pointer at all. Since its purpose is to identify running object RPC-way, I suspect it's not a simple pointer wrapper.
+static PointerMonikerImpl *unsafe_impl_from_IMoniker(IMoniker *iface) +{
- if (iface->lpVtbl != &VT_PointerMonikerImpl && iface->lpVtbl != &VT_ObjrefMonikerImpl)
return NULL;- return CONTAINING_RECORD(iface, PointerMonikerImpl, IMoniker_iface);
+}
This implies that IsEqual() could return S_OK for mismatching moniker types, which is not backed by tests, or docs.
I guess that it could be a follow up patch. I should probably mention that this implementation works perfectly for a very large .Net based application.
Like I said, it changes the way IsEqual() works.