MS OLE questions

Ove Kaaven ovek at arcticnet.no
Tue Jul 15 01:01:20 CDT 2003


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; else
> +      if (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.





More information about the wine-devel mailing list