[FFmpeg-cvslog] r19742 - trunk/libavutil/internal.h

Ramiro Polla ramiro.polla
Thu Sep 3 17:27:35 CEST 2009


On Wed, Sep 2, 2009 at 10:05 PM, Michael Niedermayer<michaelni at gmx.at> wrote:
> On Wed, Sep 02, 2009 at 03:32:26PM +0100, M?ns Rullg?rd wrote:
>> Michael Niedermayer <michaelni at gmx.at> writes:
>>
>> > On Tue, Sep 01, 2009 at 04:16:36PM +0100, M?ns Rullg?rd wrote:
>> >> Benoit Fouet <benoit.fouet at free.fr> writes:
>> >>
>> >> > On 2009-08-30 00:44, M?ns Rullg?rd wrote:
>> >> >> ramiro <subversion at mplayerhq.hu> writes:
>> >> >>
>> >> >>> Author: ramiro
>> >> >>> Date: Sun Aug 30 00:38:48 2009
>> >> >>> New Revision: 19742
>> >> >>>
>> >> >>> Log:
>> >> >>> Add CHECKED_ALLOC macro.
>> >> >>> It works the same as CHECKED_ALLOCZ except that it does not
>> >> >>> zero the allocated memory.
>> >> >>>
>> >> >>> Modified:
>> >> >>> ? ?trunk/libavutil/internal.h
>> >> >>>
>> >> >>> Modified: trunk/libavutil/internal.h
>> >> >>> ==============================================================================
>> >> >>> --- trunk/libavutil/internal.h Sat Aug 29 23:04:18 2009 ? ? ? ?(r19741)
>> >> >>> +++ trunk/libavutil/internal.h Sun Aug 30 00:38:48 2009 ? ? ? ?(r19742)
>> >> >>> @@ -249,6 +249,15 @@ if((y)<(x)){\
>> >> >>> ?#define perror please_use_av_log_instead_of_perror
>> >> >>> ?#endif
>> >> >>>
>> >> >>> +#define CHECKED_ALLOC(p, size)\
>> >> >>> +{\
>> >> >>> + ? ?p= av_malloc(size);\
>> >> >>> + ? ?if(p==NULL && (size)!=0){\
>> >> >>> + ? ? ? ?av_log(NULL, AV_LOG_ERROR, "Cannot allocate memory.");\
>> >> >>> + ? ? ? ?goto fail;\
>> >> >>> + ? ?}\
>> >> >>> +}
>> >> >>
>> >> >> Looks like I missed some discussions... ?This should be wrapped in
>> >> >> do { } while(0) so if (foo) CHECKED_ALLOC(); else blah; can work. ?It
>> >> >> would also be nice if the label to goto were an argument. ?As it is,
>> >> >> you can only have one target per function that uses the macro.
>> >> >> Finally, it could do with some beautifying, but that's secondary.
>> >> >>
>> >> >
>> >> > it would be cool to be able to pass an avctx too:
>> >> >
>> >> > #define CHECKED_ALLOC(ctx, p, size, label) ? ? ? ? ? ? ? ? ? ? ?\
>> >> > do { ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
>> >> > ? ? p = malloc(size); ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
>> >> > ? ? if (!p && (size)){ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
>> >> > ? ? ? ? av_log(ctx, AV_LOG_ERROR, "Cannot allocate memory.\n"); \
>> >> > ? ? ? ? goto label; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
>> >> > ? ? } ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
>> >> > } while (0)
>> >>
>> >> While we're at it, shouldn't this have an FF_ prefix like other
>> >> macros?
>> >
>> > and i think
>> > FF_ALLOC_OR_GOTO_FAIL(ctx, p, size)
>> > is best
>>
>> Do you have a strong objection against passing the label as a
>> parameter?
>
> no, it just feels unneeded because i cant think of any case where we
> would use it, all cases i can think of have a single "fail" target
> that frees resources

Unneeded or not, it's just one more character if you do
FF_ALLOC_OR_GOTO(ctx, p, size, fail)
instead of
FF_ALLOC_OR_GOTO_FAIL(ctx, p, size)

and I'm not a big fan of upper case =)

Patches attached. In swscale where there is no context I left it
without context and will send a patch later.

In what order should this be applied? One depends on the other and
either will break when the other is applied first.

Ramiro Polla
-------------- next part --------------
A non-text attachment was scrubbed...
Name: checked_alloc.diff
Type: text/x-diff
Size: 21299 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-cvslog/attachments/20090903/276b735a/attachment-0002.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: libswscale_checked_alloc.diff
Type: text/x-diff
Size: 6743 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-cvslog/attachments/20090903/276b735a/attachment-0003.diff>



More information about the ffmpeg-cvslog mailing list