[FFmpeg-devel] [PATCH] Remove flexible array member from Escape 124
Mon Mar 31 20:17:43 CEST 2008
On Mon, Mar 31, 2008 at 11:07:10AM -0700, Eli Friedman wrote:
> Per the thread about my Escape 124 patch ("[FFmpeg-devel] [PATCH]
> Escape 124 (RPL) decoder rev5"), patch to avoid using a flexible array
> member, since apparently it causes issues with older compilers.
It seems mostly cleaner to me, but...
> @@ -83,23 +83,23 @@
> return 0;
> -static CodeBook* unpack_codebook(GetBitContext* gb, unsigned depth,
> +static CodeBook unpack_codebook(GetBitContext* gb, unsigned depth,
> unsigned size)
I think it would be better if the function took a CodeBook* argument
instead of having a return value.
> - if (size >= (INT_MAX - sizeof(CodeBook)) / sizeof(MacroBlock))
> - return NULL;
> - cb = av_malloc(size * sizeof(MacroBlock) + sizeof(CodeBook));
> - if (!cb)
> - return NULL;
> + if (size >= INT_MAX / sizeof(MacroBlock))
> + return cb;
> + cb.blocks = av_malloc(size ? size * sizeof(MacroBlock) : 1);
Is there any reason why calling av_malloc with 0 should be a problem?
Ah, seems you will have to do the change I suggested before to make this
And too bad that there is no av_calloc, having that kind of overflow
check maintained at some central place would be nicer...
> @@ -265,9 +265,9 @@
> cb_size = s->num_superblocks << cb_depth;
> - av_free(s->codebooks[i]);
> + av_free(s->codebooks[i].blocks);
> s->codebooks[i] = unpack_codebook(&gb, cb_depth, cb_size);
> - if (!s->codebooks[i])
> + if (!s->codebooks[i].blocks)
> return -1;
Having CodeBook* as argument to unpack_codebook also has the advantage
to allow checking the return value and you can do the av_free inside
unpack_codebook as well, following the principle of doing the free as
much as possible at the same place as the alloc.
More information about the ffmpeg-devel