On Tue, May 22, 2012 at 01:30:35PM +0200, Jordi Ortiz  wrote:
> Module: libav
> Branch: master
> Commit: c89e428ed8c2c31396af2d18cab4342b7d82958f
> 
> Author:    Jordi Ortiz <[email protected]>
> Committer: Diego Biurrun <[email protected]>
> Date:      Tue May 22 13:18:17 2012 +0200
> 
> dwt: check malloc calls
> 
> Signed-off-by: Diego Biurrun <[email protected]>
> 
> ---
> 
>  libavcodec/dwt.c |   16 +++++++++++++++-
>  1 files changed, 15 insertions(+), 1 deletions(-)
> 
> diff --git a/libavcodec/dwt.c b/libavcodec/dwt.c
> index f9577fd..675644d 100644
> --- a/libavcodec/dwt.c
> +++ b/libavcodec/dwt.c
> @@ -33,10 +33,24 @@ void ff_slice_buffer_init(slice_buffer *buf, int 
> line_count,
>      buf->line_width  = line_width;
>      buf->data_count  = max_allocated_lines;
>      buf->line        = av_mallocz(sizeof(IDWTELEM *) * line_count);
> +    if (!buf->line)
> +        return AVERROR(ENOMEM);
>      buf->data_stack  = av_malloc(sizeof(IDWTELEM *) * max_allocated_lines);
> +    if (!buf->data_stack) {
> +        av_free(buf->line);
> +        return AVERROR(ENOMEM);
> +    }
>  
> -    for (i = 0; i < max_allocated_lines; i++)
> +    for (i = 0; i < max_allocated_lines; i++) {
>          buf->data_stack[i] = av_malloc(sizeof(IDWTELEM) * line_width);
> +        if (!buf->data_stack[i]) {
> +            for (i--; i >=0; i--)
> +                av_free(buf->data_stack[i]);
> +            av_free(buf->data_stack);
> +            av_free(buf->line);
> +            return AVERROR(ENOMEM);
> +        }
> +    }
>  

returning AVERROR in a void function looks wrong to me.

Anyway, the code would be more obvious with sth like this IMO (not
tested):

diff --git a/libavcodec/dwt.c b/libavcodec/dwt.c
index 675644d..adea295 100644
--- a/libavcodec/dwt.c
+++ b/libavcodec/dwt.c
@@ -33,26 +33,26 @@ void ff_slice_buffer_init(slice_buffer *buf, int line_count,
     buf->line_width  = line_width;
     buf->data_count  = max_allocated_lines;
     buf->line        = av_mallocz(sizeof(IDWTELEM *) * line_count);
-    if (!buf->line)
-        return AVERROR(ENOMEM);
-    buf->data_stack  = av_malloc(sizeof(IDWTELEM *) * max_allocated_lines);
-    if (!buf->data_stack) {
-        av_free(buf->line);
-        return AVERROR(ENOMEM);
-    }
+    buf->data_stack  = av_mallocz(sizeof(IDWTELEM *) * max_allocated_lines);
+
+    if (!buf->line || !buf->data_stack)
+        goto fail;
 
     for (i = 0; i < max_allocated_lines; i++) {
         buf->data_stack[i] = av_malloc(sizeof(IDWTELEM) * line_width);
-        if (!buf->data_stack[i]) {
-            for (i--; i >=0; i--)
-                av_free(buf->data_stack[i]);
-            av_free(buf->data_stack);
-            av_free(buf->line);
-            return AVERROR(ENOMEM);
-        }
+        if (!buf->data_stack[i])
+            goto fail;
     }
 
     buf->data_stack_top = max_allocated_lines - 1;
+    return;
+
+fail:
+    av_freep(&buf->line);
+    if (buf->data_stack)
+        for (i = 0; i < max_allocated_lines; i++)
+            av_freep(&buf->data_stack[i]);
+    av_freep(&buf->data_stack);
 }
 

-- 
Clément B.

Attachment: pgpFtzMhdpW4p.pgp
Description: PGP signature

_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to