Re: [RESEND][PATCH] libkms/exynos: fix memory leak in error path

[Eric Engestrom <eric.engestrom@imgtec.com>]

On Wednesday, 2016-12-14 17:16:30 +0900, Seung-Woo Kim wrote:
> This patch fixes memory leak in error path of exynos_bo_create().

Indeed, thanks!
Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>

> 
> Signed-off-by: Seung-Woo Kim <sw0312.kim@samsung.com>
> ---
>  libkms/exynos.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/libkms/exynos.c b/libkms/exynos.c
> index 5de2e5a..0e97fb5 100644
> --- a/libkms/exynos.c
> +++ b/libkms/exynos.c
> @@ -88,7 +88,8 @@ exynos_bo_create(struct kms_driver *kms,
>  		pitch = (pitch + 512 - 1) & ~(512 - 1);
>  		size = pitch * ((height + 4 - 1) & ~(4 - 1));
>  	} else {
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto err_free;
>  	}
>  
>  	memset(&arg, 0, sizeof(arg));
> -- 
> 1.7.4.1
> 

However, I feel like a cleaner fix might be to simply move the
allocation to where it's used and remove the now-unnecessary
error path, ie.:

----8<----
diff --git a/libkms/exynos.c b/libkms/exynos.c
index 5de2e5a..e2c1c9f 100644
--- a/libkms/exynos.c
+++ b/libkms/exynos.c
@@ -76,10 +76,6 @@ exynos_bo_create(struct kms_driver *kms,
 		}
 	}
 
-	bo = calloc(1, sizeof(*bo));
-	if (!bo)
-		return -ENOMEM;
-
 	if (type == KMS_BO_TYPE_CURSOR_64X64_A8R8G8B8) {
 		pitch = 64 * 4;
 		size = 64 * 64 * 4;
@@ -96,7 +92,11 @@ exynos_bo_create(struct kms_driver *kms,
 
 	ret = drmCommandWriteRead(kms->fd, DRM_EXYNOS_GEM_CREATE, &arg, sizeof(arg));
 	if (ret)
-		goto err_free;
+		return ret;
+
+	bo = calloc(1, sizeof(*bo));
+	if (!bo)
+		return -ENOMEM;
 
 	bo->base.kms = kms;
 	bo->base.handle = arg.handle;
@@ -106,10 +106,6 @@ exynos_bo_create(struct kms_driver *kms,
 	*out = &bo->base;
 
 	return 0;
-
-err_free:
-	free(bo);
-	return ret;
 }
 
 static int
---->8----

Bigger change, but cleaner code IMHO.
What do you think?

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