[PATCH libdrm] tests/exynos: fix memory issues of error path in g2d test
Emil Velikov
emil.l.velikov at gmail.com
Mon Mar 20 00:08:43 UTC 2017
Hi Seung-Woo Kim,
On 16 March 2017 at 02:00, Seung-Woo Kim <sw0312.kim at samsung.com> 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 at samsung.com>
> ---
> 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.
> @@ -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.
Can you give this a try, please ?
Thanks
Emil
More information about the dri-devel
mailing list