[PATCH 0/2] drm/mediatek: Adjustments for mtk_gem_prime_vmap()

[PATCH 0/2] drm/mediatek: Adjustments for mtk_gem_prime_vmap()

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 17 Sep 2024 19:16:45 +0200

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (2):
  Avoid duplicate sg_free_table(sgt) call
  Use common error handling code

 drivers/gpu/drm/mediatek/mtk_gem.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

--
2.46.0

[PATCH 1/2] drm/mediatek: Avoid duplicate sg_free_table(sgt) call in mtk_gem_prime_vmap()

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 17 Sep 2024 18:50:37 +0200

A sg_free_table(sgt) call was immediately used after a null pointer check
for the data structure member “kvaddr” in this function implementation.
Thus use such a function call only once instead directly before the check.

This issue was transformed by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/mediatek/mtk_gem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_gem.c b/drivers/gpu/drm/mediatek/mtk_gem.c
index a172456d1d7b..9c7b7d0a3ffc 100644
--- a/drivers/gpu/drm/mediatek/mtk_gem.c
+++ b/drivers/gpu/drm/mediatek/mtk_gem.c
@@ -259,13 +259,13 @@ int mtk_gem_prime_vmap(struct drm_gem_object *obj, struct iosys_map *map)

 	mtk_gem->kvaddr = vmap(mtk_gem->pages, npages, VM_MAP,
 			       pgprot_writecombine(PAGE_KERNEL));
+	sg_free_table(sgt);
 	if (!mtk_gem->kvaddr) {
-		sg_free_table(sgt);
 		kfree(sgt);
 		kfree(mtk_gem->pages);
 		return -ENOMEM;
 	}
-	sg_free_table(sgt);
+
 	kfree(sgt);

 out:
--
2.46.0

[PATCH 2/2] drm/mediatek: Use common error handling code in mtk_gem_prime_vmap()

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 17 Sep 2024 19:06:23 +0200

Add a jump target so that a bit of exception handling can be better reused
at the end of this function implementation.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/mediatek/mtk_gem.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_gem.c b/drivers/gpu/drm/mediatek/mtk_gem.c
index 9c7b7d0a3ffc..5713156efb4e 100644
--- a/drivers/gpu/drm/mediatek/mtk_gem.c
+++ b/drivers/gpu/drm/mediatek/mtk_gem.c
@@ -251,8 +251,7 @@ int mtk_gem_prime_vmap(struct drm_gem_object *obj, struct iosys_map *map)
 	mtk_gem->pages = kcalloc(npages, sizeof(*mtk_gem->pages), GFP_KERNEL);
 	if (!mtk_gem->pages) {
 		sg_free_table(sgt);
-		kfree(sgt);
-		return -ENOMEM;
+		goto free_sgt;
 	}

 	drm_prime_sg_to_page_array(sgt, mtk_gem->pages, npages);
@@ -261,9 +260,8 @@ int mtk_gem_prime_vmap(struct drm_gem_object *obj, struct iosys_map *map)
 			       pgprot_writecombine(PAGE_KERNEL));
 	sg_free_table(sgt);
 	if (!mtk_gem->kvaddr) {
-		kfree(sgt);
 		kfree(mtk_gem->pages);
-		return -ENOMEM;
+		goto free_sgt;
 	}

 	kfree(sgt);
@@ -272,6 +270,10 @@ int mtk_gem_prime_vmap(struct drm_gem_object *obj, struct iosys_map *map)
 	iosys_map_set_vaddr(map, mtk_gem->kvaddr);

 	return 0;
+
+free_sgt:
+	kfree(sgt);
+	return -ENOMEM;
 }

 void mtk_gem_prime_vunmap(struct drm_gem_object *obj, struct iosys_map *map)
--
2.46.0

Re: [PATCH 0/2] drm/mediatek: Adjustments for mtk_gem_prime_vmap()

[AngeloGioacchino Del Regno]

Il 17/09/24 19:34, Markus Elfring ha scritto:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 17 Sep 2024 19:16:45 +0200
> 
> A few update suggestions were taken into account
> from static source code analysis.

Hello Markus,

In my opinion, the suggested changes aren't bringing any improvement, not
in readability nor performance ... nor anything else, really.

For how I see it, the code is fine as it is, but then, that all boils down
to code style preferences and nothing else.

Cheers,
angelo

> 
> Markus Elfring (2):
>    Avoid duplicate sg_free_table(sgt) call
>    Use common error handling code
> 
>   drivers/gpu/drm/mediatek/mtk_gem.c | 14 ++++++++------
>   1 file changed, 8 insertions(+), 6 deletions(-)
> 
> --
> 2.46.0
>