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

[Emil Velikov]

On 14 December 2016 at 12:07, Eric Engestrom <eric.engestrom at imgtec.com> wrote:
> 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 at imgtec.com>
>
>>
>> Signed-off-by: Seung-Woo Kim <sw0312.kim at 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)
Here we're missing the drmIoctl(... DRM_IOCTL_GEM_CLOSE ...)

Another solution is to move size/pitch calculation before the calloc.
Not sure which one will be better though.

Emil