On Mon, Mar 14, 2016 at 08:10:00AM +0300, Nikolay Sivov wrote:
> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com>
> ---
> dlls/ole32/compobj.c | 57 +++++++++++++++++++---------------------------
> dlls/ole32/tests/compobj.c | 46 ++++++++++++++++++++++---------------
> 2 files changed, 51 insertions(+), 52 deletions(-)
>
> diff --git a/dlls/ole32/compobj.c b/dlls/ole32/compobj.c
> index b095aeb..1081f98 100644
> --- a/dlls/ole32/compobj.c
> +++ b/dlls/ole32/compobj.c
> @@ -4683,10 +4683,13 @@ static ULONG Context_AddRef(Context *This)
>
> static ULONG Context_Release(Context *This)
> {
> - ULONG refs = InterlockedDecrement(&This->refs);
> - if (!refs)
> + if (!This->refs)
> + {
Since this is non-standard it needs a comment. Probably just
referring to the constructor below.
> HeapFree(GetProcessHeap(), 0, This);
> - return refs;
> + return 0;
> + }
> +
> + return InterlockedDecrement(&This->refs);
> }
>
> static HRESULT WINAPI Context_CTI_QueryInterface(IComThreadingInfo *iface, REFIID riid, LPVOID *ppv)
> @@ -4924,39 +4927,19 @@ static const IObjContextVtbl Context_Object_Vtbl =
> */
> HRESULT WINAPI CoGetObjectContext(REFIID riid, void **ppv)
> {
> - APARTMENT *apt = COM_CurrentApt();
> - Context *context;
> + IObjContext *context;
> HRESULT hr;
>
> TRACE("(%s, %p)\n", debugstr_guid(riid), ppv);
>
> *ppv = NULL;
> - if (!apt)
> - {
> - if (!(apt = apartment_find_multi_threaded()))
> - {
> - ERR("apartment not initialised\n");
> - return CO_E_NOTINITIALIZED;
> - }
> - apartment_release(apt);
> - }
> -
> - context = HeapAlloc(GetProcessHeap(), 0, sizeof(*context));
> - if (!context)
> - return E_OUTOFMEMORY;
> -
> - context->IComThreadingInfo_iface.lpVtbl = &Context_Threading_Vtbl;
> - context->IContextCallback_iface.lpVtbl = &Context_Callback_Vtbl;
> - context->IObjContext_iface.lpVtbl = &Context_Object_Vtbl;
> - context->refs = 1;
> -
> - hr = IComThreadingInfo_QueryInterface(&context->IComThreadingInfo_iface, riid, ppv);
> - IComThreadingInfo_Release(&context->IComThreadingInfo_iface);
> + hr = CoGetContextToken((ULONG_PTR*)&context);
> + if (FAILED(hr))
> + return hr;
>
> - return hr;
> + return IObjContext_QueryInterface(context, riid, ppv);
> }
>
> -
> /***********************************************************************
> * CoGetContextToken [OLE32.@]
> */
> @@ -4985,16 +4968,22 @@ HRESULT WINAPI CoGetContextToken( ULONG_PTR *token )
>
> if (!info->context_token)
> {
> - HRESULT hr;
> - IObjContext *ctx;
> + Context *context;
> +
> + context = HeapAlloc(GetProcessHeap(), 0, sizeof(*context));
> + if (!context)
> + return E_OUTOFMEMORY;
> +
> + context->IComThreadingInfo_iface.lpVtbl = &Context_Threading_Vtbl;
> + context->IContextCallback_iface.lpVtbl = &Context_Callback_Vtbl;
> + context->IObjContext_iface.lpVtbl = &Context_Object_Vtbl;
Add a comment here stating that context_token does not take a ref.
> + context->refs = 0;
>
> - hr = CoGetObjectContext(&IID_IObjContext, (void **)&ctx);
> - if (FAILED(hr)) return hr;
> - info->context_token = ctx;
> + info->context_token = &context->IObjContext_iface;
> }
>
> *token = (ULONG_PTR)info->context_token;
> - TRACE("apt->context_token=%p\n", info->context_token);
> + TRACE("context_token=%p\n", info->context_token);
>
> return S_OK;
> }
I'm wondering whether this patch could be split since it's doing
rather more than fixing a ref counting issue. Could you move the
constructor to CoGetContextToken first and then fix the refs, or would
that mess up the tests?
Huw.