[PATCH] drm: sgdma: free allocated memory if TT init fails

[PATCH] drm: sgdma: free allocated memory if TT init fails

[Alexandre Courbot]

When ttm_dma_tt_init() fails in nouveau_sgdma_create_ttm(), we should
free the previously allocated memory before returning NULL.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drm/nouveau_sgdma.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drm/nouveau_sgdma.c b/drm/nouveau_sgdma.c
index 01707e7deaf5..e4fc494d688d 100644
--- a/drm/nouveau_sgdma.c
+++ b/drm/nouveau_sgdma.c
@@ -107,7 +107,10 @@ nouveau_sgdma_create_ttm(struct ttm_bo_device *bdev,
 	else
 		nvbe->ttm.ttm.func = &nv50_sgdma_backend;
 
-	if (ttm_dma_tt_init(&nvbe->ttm, bdev, size, page_flags, dummy_read_page))
+	if (ttm_dma_tt_init(&nvbe->ttm, bdev, size, page_flags, dummy_read_page)) {
+		kfree(nvbe);
 		return NULL;
+	}
+
 	return &nvbe->ttm.ttm;
 }
-- 
2.1.3

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH] drm: sgdma: free allocated memory if TT init fails

[Ilia Mirkin]

We get this patch every few months... How about one that instead adds
a comment which explains why this kfree isn't needed :) [I'm leaving
the reason to the reader, which will force you to glance at the ttm
code.]

On Wed, Dec 10, 2014 at 1:37 PM, Alexandre Courbot <acourbot@nvidia.com> wrote:
> When ttm_dma_tt_init() fails in nouveau_sgdma_create_ttm(), we should
> free the previously allocated memory before returning NULL.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
>  drm/nouveau_sgdma.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drm/nouveau_sgdma.c b/drm/nouveau_sgdma.c
> index 01707e7deaf5..e4fc494d688d 100644
> --- a/drm/nouveau_sgdma.c
> +++ b/drm/nouveau_sgdma.c
> @@ -107,7 +107,10 @@ nouveau_sgdma_create_ttm(struct ttm_bo_device *bdev,
>         else
>                 nvbe->ttm.ttm.func = &nv50_sgdma_backend;
>
> -       if (ttm_dma_tt_init(&nvbe->ttm, bdev, size, page_flags, dummy_read_page))
> +       if (ttm_dma_tt_init(&nvbe->ttm, bdev, size, page_flags, dummy_read_page)) {
> +               kfree(nvbe);
>                 return NULL;
> +       }
> +
>         return &nvbe->ttm.ttm;
>  }
> --
> 2.1.3
>
> _______________________________________________
> Nouveau mailing list
> Nouveau@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH] drm: sgdma: free allocated memory if TT init fails

[Alexandre Courbot]

On 12/11/2014 03:47 AM, Ilia Mirkin wrote:
> We get this patch every few months... How about one that instead adds
> a comment which explains why this kfree isn't needed :) [I'm leaving
> the reason to the reader, which will force you to glance at the ttm
> code.]

ttm_dma_tt_init -> ttm_tt_destroy -> nouveau_sgdma_destroy -> kfree

Urrk.

I'm definitely not a fan of things happening this way, but let it be.

Indeed a comment would be nice to have here, let me prep a patch for 
this instead...
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH] drm: sgdma: free allocated memory if TT init fails

[Ben Skeggs]

On Thu, Dec 11, 2014 at 4:58 AM, Alexandre Courbot <acourbot@nvidia.com> wrote:
> On 12/11/2014 03:47 AM, Ilia Mirkin wrote:
>>
>> We get this patch every few months... How about one that instead adds
>> a comment which explains why this kfree isn't needed :) [I'm leaving
>> the reason to the reader, which will force you to glance at the ttm
>> code.]
I'd quite like it if people looked at a bit more context for sending
such patches......

>
>
> ttm_dma_tt_init -> ttm_tt_destroy -> nouveau_sgdma_destroy -> kfree
>
> Urrk.
..... That said..  I made the same mistake myself a few years back ;)

>
> I'm definitely not a fan of things happening this way, but let it be.
>
> Indeed a comment would be nice to have here, let me prep a patch for this
> instead...
I've picked up all three patches.

Thanks,
Ben.

>
> _______________________________________________
> Nouveau mailing list
> Nouveau@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau