[FFmpeg-devel] [PATCH 3/3] random_seed: Replace a VLA with a heap alloc

Michael Niedermayer michaelni at gmx.at
Sat Sep 8 03:19:50 CEST 2012


On Fri, Sep 07, 2012 at 01:42:44PM -0400, Derek Buitenhuis wrote:
> On 07/09/2012 1:35 PM, Michael Niedermayer wrote:
> > i would suggest
> > uint8_t tmp[120];
> > ...
> > av_assert0(sizeof(tmp) >= av_sha_size)
> > 
> > this would be simpler and completely avoid variable allocation
> 
> Wouldn't this be duplicating av_sha_size? I'm not against it, but
> in general, I am wary of duplicated numbers like this.

in some sense it would, yes
but the assert would tell us if it needs to be changed and changes
are going to be rare


> 
> BTW, why is av_sha_size not a constant/define anyway?

Because changes to the SHA implementation may need a bigger struct and
the struct is allocated by the user -> it cant be a constant/define
we want a user app to work with a new SHA with new algo and possibly
larger struct without needing to recompile the user app.
the code above is not affected by this problem because it resides
in the same lib as the sha code so it can use a constant safely

The alternative design to this would have needed dynamic allocation
inside the avsha code, which would have restricted the placement
of the context and increased code complexity and dependancies. Either
is a compromise.
Also ive some XTEA optimizations in a branch that need a bigger context
(for xtea not sha) so the core problem is quite real ...

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The real ebay dictionary, page 1
"Used only once"    - "Some unspecified defect prevented a second use"
"In good condition" - "Can be repaird by experienced expert"
"As is" - "You wouldnt want it even if you were payed for it, if you knew ..."
-------------- 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/20120908/a3ec9926/attachment.asc>


More information about the ffmpeg-devel mailing list