Re:Re: [PATCH] drm/radeon: cleanup coding style a bit

[Bernard]

发件人：Joe Perches <joe at perches.com>
发送日期：2020-04-27 01:53:06
收件人："Christian König" <christian.koenig at amd.com>,Bernard Zhao <bernard at vivo.com>,Alex Deucher <alexander.deucher at amd.com>,"David (ChunMing) Zhou" <David1.Zhou at amd.com>,David Airlie <airlied at linux.ie>,Daniel Vetter <daniel at ffwll.ch>,amd-gfx at lists.freedesktop.org,dri-devel at lists.freedesktop.org,linux-kernel at vger.kernel.org
抄送人：opensource.kernel at vivo.com
主题：Re: [PATCH] drm/radeon: cleanup coding style a bit>On Sun, 2020-04-26 at 15:18 +0200, Christian König wrote:
>> Am 26.04.20 um 15:12 schrieb Bernard Zhao:
>> > Maybe no need to check ws before kmalloc, kmalloc will check
>> > itself, kmalloc`s logic is if ptr is NULL, kmalloc will just
>> > return
>> > 
>> > Signed-off-by: Bernard Zhao <bernard at vivo.com>
>> 
>> Reviewed-by: Christian König <christian.koenig at amd.com>
>> 
>> I'm wondering why the automated scripts haven't found that one before.
>
>because this pattern is
>
>	if (foo)
>		kfree(bar);
>
>and the pattern looked for is:
>
>	if (foo)
>		kfree(foo);
>
>> > diff --git a/drivers/gpu/drm/radeon/atom.c b/drivers/gpu/drm/radeon/atom.c
>[]
>> > @@ -1211,8 +1211,7 @@ static int atom_execute_table_locked(struct atom_context *ctx, int index, uint32
>> >   	SDEBUG("<<\n");
>> >   
>> >   free:
>> > -	if (ws)
>> > -		kfree(ectx.ws);
>> > +	kfree(ectx.ws);
>> >   	return ret;
>> >   }
>
>I'm wondering if this removal is correct as the function
>is named _locked and it may be recursive or called under
>some external lock.
>
Hi
I am a little confused about this. I understand that the caller guarantees the lock protection
that we will not release the wrong pointer. And the NULL check is the same with the first check in kfree?
Maybe we do not need check twich.

Regards,
Bernard