Hi, On Wed, Jul 4, 2012 at 5:08 AM, Tomas Härdin <tomas.har...@codemill.se> wrote: > On Wed, 2012-07-04 at 12:14 +0100, Måns Rullgård wrote: >> Kostya Shishkov <kostya.shish...@gmail.com> writes: >> >> > On Wed, Jul 04, 2012 at 11:09:57AM +0100, Måns Rullgård wrote: >> >> "Ronald S. Bultje" <rsbul...@gmail.com> writes: >> >> >> >> > From: "Ronald S. Bultje" <rsbul...@gmail.com> >> >> > >> >> > --- >> >> > libavformat/mxfdec.c | 8 ++++---- >> >> > 1 file changed, 4 insertions(+), 4 deletions(-) >> >> > >> >> > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c >> >> > index dd10240..a7c1890 100644 >> >> > --- a/libavformat/mxfdec.c >> >> > +++ b/libavformat/mxfdec.c >> >> > @@ -700,7 +700,7 @@ static int mxf_read_index_entry_array(AVIOContext >> >> > *pb, MXFIndexTableSegment *seg >> >> > return 0; >> >> > else if (segment->nb_index_entries < 0 || >> >> > segment->nb_index_entries > >> >> > - (INT_MAX >> >> >> > av_log2(sizeof(*segment->stream_offset_entries)))) >> >> > + (INT_MAX / sizeof(*segment->stream_offset_entries))) >> >> > return AVERROR(ENOMEM); >> >> > >> >> > length = avio_rb32(pb); >> >> > @@ -1084,7 +1084,7 @@ static int >> >> > mxf_compute_ptses_fake_index(MXFContext *mxf, MXFIndexTable *index_ta >> >> > if (index_table->nb_ptses <= 0) >> >> > return 0; >> >> > >> >> > - if (index_table->nb_ptses > INT_MAX >> >> >> > av_log2(sizeof(AVIndexEntry)) + 1) >> >> > + if (index_table->nb_ptses >= INT_MAX / sizeof(AVIndexEntry)) >> >> > return AVERROR(ENOMEM); >> >> >> >> What happened to the +1? >> > >> > merged with > to make >= ? >> >> But that's not equivalent. It's INT_MAX >> (foo + 1), so the equivalent >> division is INT_MAX / (2 * bar). > > Why use a shift in the first place? Anyway, since it's checking that an > allocation is < INT_MAX a normal division should work well enough. > x*(INT_MAX/x) <= INT_MAX. So >=. > > Why not simply add av_calloc() to lavu and use it? These kind of > allocations are all over the place - it'd make them prettier. I've > suggested this before in [1]. You could even have it static inline. > > IMO muxers shouldn't have to care about whatever the maximum allocation > size is. That would also allow changing said size more easily (it'd be > confined to mem.*) > > [1] http://www.mail-archive.com/libav-devel@libav.org/msg16665.html
I'd say send a patch? Also, it'd be faster if av_calloc() were inlined in a header file, since then the division (INT_MAX / sizeof(..)) can be done by the compiler. Ronald _______________________________________________ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
