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

[Nikolay Sivov]

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'.
> That said that would be good to have some recommendations so people 
> can use follow them.
True.
>
>