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

[lance.lmwang at gmail.com]

On Tue, May 12, 2020 at 11:49:01AM +0200, Nicolas George wrote:
> 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.
No, it's not hard-coded, you can name your own goto label.

So below is my change by your proposal:
 #define FF_ALLOC_ARRAY_OR_GOTO(p, nelem, elsize, ret, label)\
 {\
     p = av_malloc_array(nelem, elsize);\
     if (!p) {\
         ret = AVERROR(ENOMEM); \
         goto label;
     }\
 }


> 
> 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



-- 
Thanks,
Limin Wang