[FFmpeg-devel] [PATCH] avutil/internal: add FF_ALLOC_ARRAY_OR_GOTO & FF_ALLOCZ_ARRAY_OR_GOTO

[Michael Niedermayer]

On Sat, Apr 19, 2014 at 12:01:00AM +0200, Nicolas George wrote:
> Le nonidi 29 germinal, an CCXXII, Michael Niedermayer a écrit :
> > These are similar to the existing FF_ALLOCZ_OR_GOTO & FF_ALLOC_OR_GOTO
> > 
> > Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> > ---
> >  libavutil/internal.h |   18 ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> > 
> > diff --git a/libavutil/internal.h b/libavutil/internal.h
> > index 9c5546f..05a017f 100644
> > --- a/libavutil/internal.h
> > +++ b/libavutil/internal.h
> > @@ -133,6 +133,24 @@
> >      }\
> >  }
> >  
> > +#define FF_ALLOC_ARRAY_OR_GOTO(ctx, p, size0, size1, label)\
> > +{\
> > +    p = av_malloc(size0, size1);\
> > +    if (p == NULL) {\
> > +        av_log(ctx, AV_LOG_ERROR, "Cannot allocate memory.\n");\
> > +        goto label;\
> > +    }\
> > +}
> 

> A short doxy would be nice. Or at least, naming the arguments more
> explicitly than size0 and size1. Maybe nelem and elsize, like in SUS.

ok, ill make that change


> 
> A lot of places would require some kind of "ret = AVERROR(ENOMEM);" before
> jumping to the failure label.
> 
> I am not sure about the unconditional log. Especially for OOM: the error
> message is completely redundant with AVERROR(ENOMEM), and someone higher in
> the call graph will probably translate the returned error code and print it.
> That will print the error message twice.

I wanted to replace existing FF_ALLOC_OR_GOTO by this where
appropriate, and for that it must use the same API as FF_ALLOC_OR_GOTO
otherwise it wont mix well with unchanged surrounding FF_ALLOC_OR_GOTO
code.


> 
> (I still hope someone will come up with something brilliant to make error
> handling simpler and more practical. Or at least more consistent. Maybe
> thread-local storage can help. Or longjmp(). Or storing things in AVClass
> objects. I do not know. I like the way Gtk+ works, with a GError** extra
> argument to return the information. Also, a way of knowing the point in the
> code that generated the error would be nice, at least for debug builds.)
> 
> Regards,
> 
> -- 
>   Nicolas George



> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who are too smart to engage in politics are punished by being
governed by those who are dumber. -- Plato 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140419/35b9b9c9/attachment.asc>