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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 30.04.20 um 13:00 schrieb Bernard:

发件人:Joe Perches <joe@xxxxxxxxxxx>
发送日期:2020-04-27 01:53:06
收件人:"Christian König" <christian.koenig@xxxxxxx>,Bernard Zhao <bernard@xxxxxxxx>,Alex Deucher <alexander.deucher@xxxxxxx>,"David (ChunMing) Zhou" <David1.Zhou@xxxxxxx>,David Airlie <airlied@xxxxxxxx>,Daniel Vetter <daniel@xxxxxxxx>,amd-gfx@xxxxxxxxxxxxxxxxxxxxx,dri-devel@xxxxxxxxxxxxxxxxxxxxx,linux-kernel@xxxxxxxxxxxxxxx
抄送人:opensource.kernel@xxxxxxxx
主题: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@xxxxxxxx>
Reviewed-by: Christian König <christian.koenig@xxxxxxx>

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.

I don't understand the comment either. When you look at the function you see that code is freeing up the temporary allocated buffer which is to large for the stack.

In other words we kcalloc() this buffer a few lines above and free it here again. So I think the patch is perfectly valid.

What we could do is to update the coci pattern to catch this as well, but this case is so rare that it is probably not worth it.

Regards,
Christian.


Regards,
Bernard



_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux