Hello Emil,
Thanks for comment.
On 2017년 03월 20일 09:08, Emil Velikov wrote:
> Hi Seung-Woo Kim,
>
> On 16 March 2017 at 02:00, Seung-Woo Kim <sw0312.kim@xxxxxxxxxxx> wrote:
>> This patch fixes memory issues including NULL deference and leak
>> in g2d test in error path.
>>
>> Signed-off-by: Seung-Woo Kim <sw0312.kim@xxxxxxxxxxx>
>> ---
>> tests/exynos/exynos_fimg2d_test.c | 13 +++++++------
>> 1 files changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/tests/exynos/exynos_fimg2d_test.c b/tests/exynos/exynos_fimg2d_test.c
>> index 797fb6e..2177e08 100644
>> --- a/tests/exynos/exynos_fimg2d_test.c
>> +++ b/tests/exynos/exynos_fimg2d_test.c
>> @@ -59,7 +59,6 @@ static void connector_find_mode(int fd, struct connector *c,
>> if (!connector) {
>> fprintf(stderr, "could not get connector %i: %s\n",
>> resources->connectors[i], strerror(errno));
>> - drmModeFreeConnector(connector);
>> continue;
>> }
>>
>> @@ -98,7 +97,6 @@ static void connector_find_mode(int fd, struct connector *c,
>> if (!c->encoder) {
>> fprintf(stderr, "could not get encoder %i: %s\n",
>> resources->encoders[i], strerror(errno));
>> - drmModeFreeEncoder(c->encoder);
>> continue;
>> }
>>
>> @@ -264,7 +262,8 @@ static int g2d_copy_test(struct exynos_device *dev, struct exynos_bo *src,
>> userptr = (unsigned long)malloc(size);
>> if (!userptr) {
>> fprintf(stderr, "failed to allocate userptr.\n");
>> - return -EFAULT;
>> + ret = -EFAULT;
>> + goto fail;
> Even if you have the best of intentions, but there's yet another bug
> in the existing code :-\
>
> Namely: we always return 0 even on error - i.e. the "return 0", after
> the g2d_fini() must be "return ret"
> This applies to all the tests, afaics.
You are right. I will fix it.
>
>
>> @@ -755,7 +756,7 @@ int main(int argc, char **argv)
>>
>> dev = exynos_device_create(fd);
>> if (!dev) {
>> - drmClose(dev->fd);
>> + drmClose(fd);
> Seems correct, but an alternative (better IMHO) solution is to:
> - flip/fix the call drmClose() <> exynos_device_destroy() order in
> err_drm_close.
> - use "fd" in exynos_device_destroy's drmClose.
> - add separate label and use it in the above case.
Ok, I will add error label.
>
> Can you give this a try, please ?
After fixing as your comment, I will send v2.
Regards,
- Seung-Woo Kim
>
> Thanks
> Emil
>
>
>
--
Seung-Woo Kim
Samsung Software R&D Center
--
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
