[PATCH 8/9] d3drm: Add refcount info to AddRed and Release traces.

Christian Costa titan.costa at gmail.com
Fri Mar 9 06:17:29 CST 2012


Le 09/03/2012 13:49, Nikolay Sivov a écrit :
> On 3/9/2012 14:46, Christian Costa wrote:
>> Le 09/03/2012 13:06, Nikolay Sivov a écrit :
>>> On 3/9/2012 13:56, Christian Costa wrote:
>>>>
>>>> diff --git a/dlls/d3drm/d3drm.c b/dlls/d3drm/d3drm.c
>>>> index 879d8b0..42d722c 100644
>>>> --- a/dlls/d3drm/d3drm.c
>>>> +++ b/dlls/d3drm/d3drm.c
>>>> @@ -87,10 +87,11 @@ static HRESULT WINAPI 
>>>> IDirect3DRMImpl_QueryInterface(IDirect3DRM* iface, REFIID
>>>>   static ULONG WINAPI IDirect3DRMImpl_AddRef(IDirect3DRM* iface)
>>>>   {
>>>>       IDirect3DRMImpl *This = impl_from_IDirect3DRM(iface);
>>>> +    ULONG ref = InterlockedIncrement(&This->ref);
>>>>
>>>> -    TRACE("(%p/%p)\n", iface, This);
>>>> +    TRACE("(%p/%p): AddRef from %d\n", iface, This, ref - 1);
>>>>
>>>> -    return InterlockedIncrement(&This->ref);
>>>> +    return ref;
>>>>   }
>>>>
>>>>   static ULONG WINAPI IDirect3DRMImpl_Release(IDirect3DRM* iface)
>>>> @@ -98,7 +99,7 @@ static ULONG WINAPI 
>>>> IDirect3DRMImpl_Release(IDirect3DRM* iface)
>>>>       IDirect3DRMImpl *This = impl_from_IDirect3DRM(iface);
>>>>       ULONG ref = InterlockedDecrement(&This->ref);
>>>>
>>>> -    TRACE("(%p/%p)\n", iface, This);
>>>> +    TRACE("(%p/%p): ReleaseRef to %d\n", iface, This, ref);
>>>>
>>>>       if (!ref)
>>>>           HeapFree(GetProcessHeap(), 0, This);
>>>> diff --git a/dlls/d3drm/device.c b/dlls/d3drm/device.c
>>>> index 559128e..6f9802a 100644
>>>> --- a/dlls/d3drm/device.c
>>>> +++ b/dlls/d3drm/device.c
>>>> @@ -93,10 +93,11 @@ static HRESULT WINAPI 
>>>> IDirect3DRMDevice2Impl_QueryInterface(IDirect3DRMDevice2*
>>>>   static ULONG WINAPI 
>>>> IDirect3DRMDevice2Impl_AddRef(IDirect3DRMDevice2* iface)
>>>>   {
>>>>       IDirect3DRMDeviceImpl *This = 
>>>> impl_from_IDirect3DRMDevice2(iface);
>>>> +    ULONG ref = InterlockedIncrement(&This->ref);
>>>>
>>>> -    TRACE("(%p)\n", This);
>>>> +    TRACE("(%p): AddRef from %d\n", This, ref - 1);
>>>>
>>>> -    return InterlockedIncrement(&This->ref);
>>>> +    return ref;
>>>>   }
>>>>
>>> I personally don't see a reason to trace actual reference on release 
>>> and previous one on AddRef. I usually trace new value in both calls 
>>> so you don't need to keep in mind this +/- 1 offset when looking at 
>>> logs.
>>>
>>> Also you don't need extra words in trace message cause function name 
>>> is visible in traces already.
>>>
>>>
>> Indeed that does not make sense but I've a always seen that. In 
>> addition I took the message from another place.
>> I don't mind changing at all. I like "(%p)->(): new ref = %d\n" and I 
>> will stick to it.
> This is too verbose too, format like (%p)->(%d) is enough. You don't 
> usually look at refcount traces while debugging, but when you see it 
> traced value should be consistent so you don't need to think is it 
> 'new ref' or 'old ref'.
->(...) are for arguments and I would keep 'new' to avoid confusion. 
Maybe I could remove 'ref' but hey I don't think it's so verbose, it 
take one line anyway.
I never have problem with refcount traces. This is the first thing I 
look at when I debug a crash. Using a dedicated channel for refcount 
would be better to reduce verbosity IMO.




More information about the wine-devel mailing list