diff mbox

[FFmpeg-devel] fix mem-leak

Message ID 1520836255-9581-1-git-send-email-danyaschenko@gmail.com
State New
Headers show

Commit Message

Danil Iashchenko March 12, 2018, 6:30 a.m. UTC 
---
 libavfilter/vf_convolution_opencl.c | 39 ++++++++++++++-----------------------
 1 file changed, 15 insertions(+), 24 deletions(-)

Comments

Mark Thompson March 12, 2018, 1:11 p.m. UTC | #1 
On 12/03/18 06:30, Danil Iashchenko wrote:
> ---
>  libavfilter/vf_convolution_opencl.c | 39 ++++++++++++++-----------------------
>  1 file changed, 15 insertions(+), 24 deletions(-)
> 
> diff --git a/libavfilter/vf_convolution_opencl.c b/libavfilter/vf_convolution_opencl.c
> index 60e2d1f..a14005b 100644
> --- a/libavfilter/vf_convolution_opencl.c
> +++ b/libavfilter/vf_convolution_opencl.c
> @@ -139,7 +139,7 @@ static int convolution_opencl_make_filter_params(AVFilterContext *avctx)
>      matrix = av_malloc(matrix_bytes);
>      if (!matrix) {
>          err = AVERROR(ENOMEM);
> -        goto fail;
> +        goto fail_matrix;
>      }
>      cnt = 0;
>      for (i = 0; i < 4; i++) {
> @@ -157,7 +157,7 @@ static int convolution_opencl_make_filter_params(AVFilterContext *avctx)
>          av_log(avctx, AV_LOG_ERROR, "Failed to create matrix buffer: "
>                 "%d.\n", cle);
>          err = AVERROR(EIO);
> -        goto fail;
> +        goto fail_matrix;
>      }
>      ctx->matrix = buffer;
>  
> @@ -166,8 +166,7 @@ static int convolution_opencl_make_filter_params(AVFilterContext *avctx)
>      matrix_rdiv = av_malloc(matrix_bytes);
>      if (!matrix_rdiv) {
>          err = AVERROR(ENOMEM);
> -        av_freep(&matrix_rdiv);
> -        goto fail;
> +        goto fail_rdiv;
>      }
>  
>      for (j = 0; j < 4; j++) {
> @@ -182,8 +181,7 @@ static int convolution_opencl_make_filter_params(AVFilterContext *avctx)
>          av_log(avctx, AV_LOG_ERROR, "Failed to create rdiv buffer: "
>                 "%d.\n", cle);
>          err = AVERROR(EIO);
> -        av_freep(&matrix_rdiv);
> -        goto fail;
> +        goto fail_rdiv;
>      }
>      ctx->rdiv_buffer = buffer_rdiv;
>  
> @@ -192,9 +190,7 @@ static int convolution_opencl_make_filter_params(AVFilterContext *avctx)
>      matrix_bias = av_malloc(matrix_bytes);
>      if (!matrix_bias) {
>          err = AVERROR(ENOMEM);
> -        av_freep(&matrix_rdiv);
> -        av_freep(&matrix_bias);
> -        goto fail;
> +        goto fail_bias;
>      }
>  
>      for (j = 0; j < 4; j++) {
> @@ -209,9 +205,7 @@ static int convolution_opencl_make_filter_params(AVFilterContext *avctx)
>          av_log(avctx, AV_LOG_ERROR, "Failed to create bias buffer: "
>                 "%d.\n", cle);
>          err = AVERROR(EIO);
> -        av_freep(&matrix_rdiv);
> -        av_freep(&matrix_bias);
> -        goto fail;
> +        goto fail_bias;
>      }
>      ctx->bias_buffer = buffer_bias;
>  
> @@ -219,10 +213,7 @@ static int convolution_opencl_make_filter_params(AVFilterContext *avctx)
>      matrix_size = av_malloc(matrix_bytes);
>      if (!matrix_size) {
>          err = AVERROR(ENOMEM);
> -        av_freep(&matrix_rdiv);
> -        av_freep(&matrix_bias);
> -        av_freep(&matrix_size);
> -        goto fail;
> +        goto fail_size;
>      }
>  
>      for (j = 0; j < 4; j++) {
> @@ -237,18 +228,18 @@ static int convolution_opencl_make_filter_params(AVFilterContext *avctx)
>          av_log(avctx, AV_LOG_ERROR, "Failed to create bias buffer: "
>                 "%d.\n", cle);
>          err = AVERROR(EIO);
> -        av_freep(&matrix_rdiv);
> -        av_freep(&matrix_bias);
> -        av_freep(&matrix_size);
> -        goto fail;
> +        goto fail_size;
>      }
>      ctx->size_buffer = buffer_size;
>  
> -
> -
> -
>      err = 0;
> -fail:
> +fail_size:
> +    av_freep(&matrix_size);
> +fail_bias:
> +    av_freep(&matrix_bias);
> +fail_rdiv:
> +    av_freep(&matrix_rdiv);
> +fail_matrix:
>      av_freep(&matrix);
>      return err;
>  }
> 

av_freep(&pointer) is a no-op if pointer is NULL (just like normal free()).  As such, if you initialise these matrix values to NULL at the start of the function then you can call av_freep() on them unconditionally and avoid having the separate fail_foo labels.

Also, I think it is preferred to send the whole patch again with changes like this squashed into it (maybe with a -v N to distinguish successive versions) rather than sending patches on top of the patch.

(I'll look at the whole thing properly later today.)

Thanks,

- Mark
diff mbox

Patch

diff --git a/libavfilter/vf_convolution_opencl.c b/libavfilter/vf_convolution_opencl.c
index 60e2d1f..a14005b 100644
--- a/libavfilter/vf_convolution_opencl.c
+++ b/libavfilter/vf_convolution_opencl.c
@@ -139,7 +139,7 @@  static int convolution_opencl_make_filter_params(AVFilterContext *avctx)
     matrix = av_malloc(matrix_bytes);
     if (!matrix) {
         err = AVERROR(ENOMEM);
-        goto fail;
+        goto fail_matrix;
     }
     cnt = 0;
     for (i = 0; i < 4; i++) {
@@ -157,7 +157,7 @@  static int convolution_opencl_make_filter_params(AVFilterContext *avctx)
         av_log(avctx, AV_LOG_ERROR, "Failed to create matrix buffer: "
                "%d.\n", cle);
         err = AVERROR(EIO);
-        goto fail;
+        goto fail_matrix;
     }
     ctx->matrix = buffer;
 
@@ -166,8 +166,7 @@  static int convolution_opencl_make_filter_params(AVFilterContext *avctx)
     matrix_rdiv = av_malloc(matrix_bytes);
     if (!matrix_rdiv) {
         err = AVERROR(ENOMEM);
-        av_freep(&matrix_rdiv);
-        goto fail;
+        goto fail_rdiv;
     }
 
     for (j = 0; j < 4; j++) {
@@ -182,8 +181,7 @@  static int convolution_opencl_make_filter_params(AVFilterContext *avctx)
         av_log(avctx, AV_LOG_ERROR, "Failed to create rdiv buffer: "
                "%d.\n", cle);
         err = AVERROR(EIO);
-        av_freep(&matrix_rdiv);
-        goto fail;
+        goto fail_rdiv;
     }
     ctx->rdiv_buffer = buffer_rdiv;
 
@@ -192,9 +190,7 @@  static int convolution_opencl_make_filter_params(AVFilterContext *avctx)
     matrix_bias = av_malloc(matrix_bytes);
     if (!matrix_bias) {
         err = AVERROR(ENOMEM);
-        av_freep(&matrix_rdiv);
-        av_freep(&matrix_bias);
-        goto fail;
+        goto fail_bias;
     }
 
     for (j = 0; j < 4; j++) {
@@ -209,9 +205,7 @@  static int convolution_opencl_make_filter_params(AVFilterContext *avctx)
         av_log(avctx, AV_LOG_ERROR, "Failed to create bias buffer: "
                "%d.\n", cle);
         err = AVERROR(EIO);
-        av_freep(&matrix_rdiv);
-        av_freep(&matrix_bias);
-        goto fail;
+        goto fail_bias;
     }
     ctx->bias_buffer = buffer_bias;
 
@@ -219,10 +213,7 @@  static int convolution_opencl_make_filter_params(AVFilterContext *avctx)
     matrix_size = av_malloc(matrix_bytes);
     if (!matrix_size) {
         err = AVERROR(ENOMEM);
-        av_freep(&matrix_rdiv);
-        av_freep(&matrix_bias);
-        av_freep(&matrix_size);
-        goto fail;
+        goto fail_size;
     }
 
     for (j = 0; j < 4; j++) {
@@ -237,18 +228,18 @@  static int convolution_opencl_make_filter_params(AVFilterContext *avctx)
         av_log(avctx, AV_LOG_ERROR, "Failed to create bias buffer: "
                "%d.\n", cle);
         err = AVERROR(EIO);
-        av_freep(&matrix_rdiv);
-        av_freep(&matrix_bias);
-        av_freep(&matrix_size);
-        goto fail;
+        goto fail_size;
     }
     ctx->size_buffer = buffer_size;
 
-
-
-
     err = 0;
-fail:
+fail_size:
+    av_freep(&matrix_size);
+fail_bias:
+    av_freep(&matrix_bias);
+fail_rdiv:
+    av_freep(&matrix_rdiv);
+fail_matrix:
     av_freep(&matrix);
     return err;
 }