[FFmpeg-devel] [PATCH v2 1/4] avutil/internal: add FF_ALLOC_TYPED_ARRAY_OR_GOTO & FF_ALLOCZ_TYPED_ARRAY_OR_GOTO

[Nicolas George]

lance.lmwang at gmail.com (12020-05-11):
> From: Limin Wang <lance.lmwang at gmail.com>
> 
> These are similar to the existing FF_ALLOC_ARRAY_OR_GOTO & FF_ALLOCZ_ARRAY_OR_GOTO,
> but the elsize is calcuated by sizeof(*p)
> 
> Signed-off-by: Limin Wang <lance.lmwang at gmail.com>
> ---
>  libavutil/internal.h | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/libavutil/internal.h b/libavutil/internal.h
> index 4acbcf5..1be9001 100644
> --- a/libavutil/internal.h
> +++ b/libavutil/internal.h
> @@ -173,6 +173,24 @@
>      }\
>  }
>  
> +#define FF_ALLOC_TYPED_ARRAY_OR_GOTO(ctx, p, nelem, label)\
> +{\
> +    p = av_malloc_array(nelem, sizeof(*p));\
> +    if (!p) {\
> +        av_log(ctx, AV_LOG_ERROR, "Cannot allocate memory.\n");\
> +        goto label;\
> +    }\
> +}
> +
> +#define FF_ALLOCZ_TYPED_ARRAY_OR_GOTO(ctx, p, nelem, label)\
> +{\
> +    p = av_mallocz_array(nelem, sizeof(*p));\
> +    if (!p) {\
> +        av_log(ctx, AV_LOG_ERROR, "Cannot allocate memory.\n");\
> +        goto label;\
> +    }\
> +}
> +
>  #include "libm.h"
>  
>  /**

Please NO!

These functions have a terrible design, let us fix them before extending
them.

First design mistake: no error code. A helper function for testing
memory allocation failure where AVERROR(ENOMEM) does not appear is
absurd.

Second design mistake: printing a message. Return the error code, let
the caller print the error message.

Third design mistake: hard-coded use of goto.

Best design:

#define FF_ALLOC_ARRAY_OR_GOTO(p, nelem, elsize, ret, error)\
{\
    p = av_malloc_array(nelem, elsize);\
    if (!p) {\
        ret = AVERROR(ENOMEM); \
        error;
    }\
}

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200512/8ebe899e/attachment.sig>