On 14 December 2016 at 13:12, Eric Engestrom <eric.engestrom@xxxxxxxxxx> wrote:
> On Wednesday, 2016-12-14 12:21:55 +0000, Emil Velikov wrote:
>> On 14 December 2016 at 12:07, Eric Engestrom <eric.engestrom@xxxxxxxxxx> 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@xxxxxxxxxx>
>> >
>> >>
>> >> Signed-off-by: Seung-Woo Kim <sw0312.kim@xxxxxxxxxxx>
>> >> ---
>> >> 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 ...)
>
> You're right... I'm not actually improving anything with my suggestion.
>
>>
>> Another solution is to move size/pitch calculation before the calloc.
>> Not sure which one will be better though.
>
> Let's just land Seung-Woo's fix?
> We can always refactor later :)
Agreed. it was just an idea for the future.
Thanks gents, the patch is in master now.
-Emil
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
