man, 14.07.2003 kl. 17.55 skrev Mike Hearn:
Firstly, how much of the code in this patch is already duplicated in the WineHQ tree? I see a oleproxy.c file, from Marcus (C) 2002, which you removed from the build, but your patch is in parts (C) 2001. Does that mean that the patch removes code that actually it shouldn't when applied to WineHQ?
oleproxy.c and related files are both LGPL and obsolete (their functionality is supposed to handled by rpcrt4 and IDL tools), therefore I don't plan to touch it in my implementation. I use my own code instead, which happened to use a different file name. Whenever my patches are merged into WineHQ, the ole32/oleproxy.c and friends is likely to be either removed or replaced with chunks from my proxy.c (I'm not sure which). But as I explained in the mail, I assume the case will be different in oleaut32/, where the existing LGPL-ed stuff isn't all that obsolete (yet? its functionality should probably also really be in rpcrt4 but there's no reason to move it there at this time), so files there will probably just be adapted for the RPC transport, not replaced with my X11-licensed code like in my patch.
I don't understand this code:
+static void RpcChannel_push_request(RpcRequest *req) +{
- req->next = NULL;
- req->ret = RPC_S_CALL_IN_PROGRESS; /* ? */
- EnterCriticalSection(&creq_cs);
- if (creq_tail) creq_tail->next = req;
- else {
- creq_head = req;
- creq_tail = req;
- }
- LeaveCriticalSection(&creq_cs);
+}
If this is a double-ended queue, like it looks, then shouldn't the middle line read:
if (creq_tail) { creq_tail->next = req; creq_tail = req; } else .....
Hmm, maybe you're right. I copied it from dlls/rpcrt4/rpc_server.c where there's a RPCRT4_push_packet with the same bug, then. Still my fault as that was written by me too...
I don't understand why StubMan_Invoke only appears able to marshal IRemUnknown - I've been reading chapter 5 of "Essential COM" as well as MSDN and it would seem it should be able to marshal any interface? Is that just time pressures?
StubMan is what manages the object's lifetime (reference counts), it is the server-side thing that only handles IUnknown (or IRemUnknown in my implementation, which is technically wrong, as IRemUnknown should be handled by the apartment, not the stub manager). For other interfaces, the respective interface stub's Invoke is called instead; StubMan_Invoke don't have to know about them. You can see the logic of finding out which Invoke to invoke in COM_RpcDispatch. The StubMan's own Invoke is only used for IRemUnknown.
I don't understand what COM_CreateIIf does. Creates an imported interface?
Yes, kind of. Basically instantiate (and keep track of) an interface proxy, so that it's possible to use the requested interface from the apartment that imported the object. The interface proxy (created by COM_CreateIIf and returned to the app when it wants the object) will marshal calls to its methods and transmit them through rpcrt4 (wrapped by IRpcChannelBuffer), which will hand it over to COM_RpcDispatch, which will send the request to the exporting apartment and have its interface stub (created by COM_CreateXIf) unmarshal the data, call the function (in its own apartment), marshal the result (all done by calling its Invoke method), the result again goes through rpcrt4, and the interface proxy can then unmarshal the result and return to its caller. The book should have more detailed information about this process, but that's what this implements.
In CoMarshalInterThreadInterfaceInStream(), there is this code:
+#ifdef FAKE_INTERTHREAD
- hr = IStream_Write(*ppStm, &pUnk, sizeof(pUnk), NULL);
- if (SUCCEEDED(hr)) IUnknown_AddRef(pUnk);
- TRACE("<= %p\n", pUnk);
+#else
- hr = CoMarshalInterface(*ppStm, riid, pUnk, MSHCTX_INPROC, 0, MSHLFLAGS_NORMAL);
+#endif
Was that just for development, or are there still times when fake interthread marshalling is needed?
No, there aren't times anymore. That was used when we had interprocess marshalling but not interthread.
In the builtin Proxy/Stub section, for IClassFactory, there is this code:
+#if 0 +/* we need to create an IDL compiler for Wine that can generate
- most of these automatically */
Followed by a section of unused code. Was this written before you decided to use MIDL?
Not before I "decided", but before I had written all the rpcrt4 code needed for using MIDL-generated code.
Is this code in the auto-generated dcom marshalling code? (I didn't read all of the auto-genned stuf)
It should be in unknwn_p.c
In your typelib marshaller, there is this code:
- case VT_VOID: /* <= InstallShield hack */
- {
LPVOID pv = *(LPVOID*)args;const IID *piid;if (pType->vt == VT_DISPATCH) piid = &IID_IDispatch; elseif (pType->vt == VT_UNKNOWN) piid = &IID_IUnknown; else{pv = (LPVOID)args;piid = is_iid;}TRACE(" marshaling INTERFACE %p %s\n", pv, debugstr_guid(piid));hr = CoMarshalInterface(pStm, piid, pv, CLSCTX_LOCAL_SERVER, NULL, MSHLFLAGS_NORMAL);- }
- break;
but in Marcus', it is just this:
case VT_VOID:if (debugout) MESSAGE("<void>"); return S_OK;
He might be handling it at a higher level, then, as he do have some "thisisiid" stuff.
What is supposed to happen when marshalling a void param like this?
It's not legal for an app to do it. However, because MIDL-based marshallers weren't working before this patch, we had to force installshield to use the typelib marshaller (even though it didn't want to), and then hack our typelib marshallers to handle what installshield wanted to do. Treating VT_VOID as an interface (like VT_UNKNOWN) was enough to make installshield work without using its MIDL-based marshallers. The "force" is the "if (1 ||" at line 525 in typelib.c, the "1 || " can be removed after my patch is applied, and then the above VT_VOID hacks could be removed too, since then the MIDL-based marshallers would be used instead of the typelib marshaller to handle the interfaces that has VT_VOIDs in them.
+static HRESULT WINAPI PSOAStub_Invoke(LPRPCSTUBBUFFER iface,
PRPCOLEMESSAGE pMsg,LPRPCCHANNELBUFFER pChannel)This function looks a lot like WineHQs ITypeInfo::Invoke. Is there any relation?
ITypeInfo::Invoke converts a list of variants to suitable raw function arguments and then calls the method with it. My PSOAStub_Invoke has already unmarshalled the data into suitable raw function arguments so it doesn't need this conversion, can just call the method directly. But it would probably be *possible* to implement PSOAStub_Invoke in terms of ITypeInfo::Invoke, by unmarshalling into a variant list instead of raw function arguments.
Or did you mean to compare it to TMStubImpl_Invoke?
+static LPVOID OA_BuildProxyVtbl(LPTYPEINFO pInfo, int *pfs, int rec)
Likewise, this function would seem to be similar but not the same as some code in WineHQ (they both give warnings if you don't have stdole32.tlb). Same? Different?
You mean PSFacBuf_CreateProxy? Yes, basically accomplishes largely the same thing. As mentioned, Marcus's oleaut32 code doesn't really have to be removed and replaced, just adapted. I just don't use it, because of the LGPL-ness of patches to it and such.
Thanks for any insight you can give to me on these questions. Finally, I am wondering whether anything would break if I simply merged in (to my local tree) the duplicated code.
I don't know, I haven't tried to apply it to a current Wine tree. Might depend on how you handle the merging (for example, whether you'll just use Marcus's tmarshal code and change it to work with rpc, and if so, whether you do the changes needed to do that correctly.)
As I mentioned previously, time is a big problem here, and I can't use Microsofts own implementation for whatever reason. I don't care about cleanliness.
Well, good luck.