[FFmpeg-devel] [RFC] consistify/simplify/whatever index validation in vorbis_dec
Reimar Döffinger
Reimar.Doeffinger
Wed Sep 23 17:27:16 CEST 2009
Hello,
This adds a VALIDATE_INDEX macro that does the typical index validation
necessary for vorbis all over the place and makes the checks and
messages for it consistent.
Michael suggested making the check and the message an argument, but my
opinion is that these case are so much alike it does not really improve
things but adds complexity and bloat.
I even removed the argument for what to do on error to just alway return
-1.
This makes things a bit annoying if someone wants to improve the checks
to e.g. do something more intelligent than aborting, but I don't really
see that happen and thus consider this approach appropriate.
-------------- next part --------------
Index: libavcodec/vorbis_dec.c
===================================================================
--- libavcodec/vorbis_dec.c (revision 20001)
+++ libavcodec/vorbis_dec.c (working copy)
@@ -163,6 +163,15 @@
#define BARK(x) \
(13.1f*atan(0.00074f*(x))+2.24f*atan(1.85e-8f*(x)*(x))+1e-4f*(x))
+
+#define VALIDATE_INDEX(idx, limit) \
+ if (idx >= limit) {\
+ av_log(vc->avccontext, AV_LOG_ERROR,\
+ "Index value %d out of range (0 - %d) for %s at %s:%i\n",\
+ (int)(idx), (int)(limit - 1), #idx, __FILE__, __LINE__);\
+ return -1;\
+ }
+
static float vorbisfloat2float(uint_fast32_t val) {
double mant=val&0x1fffff;
long exp=(val&0x7fe00000L)>>21;
@@ -488,23 +497,16 @@
AV_DEBUG(" %d floor %d class dim: %d subclasses %d \n", i, j, floor_setup->data.t1.class_dimensions[j], floor_setup->data.t1.class_subclasses[j]);
if (floor_setup->data.t1.class_subclasses[j]) {
- int bits=get_bits(gb, 8);
- if (bits>=vc->codebook_count) {
- av_log(vc->avccontext, AV_LOG_ERROR, "Masterbook index %d is out of range.\n", bits);
- return -1;
- }
- floor_setup->data.t1.class_masterbook[j]=bits;
+ floor_setup->data.t1.class_masterbook[j]=get_bits(gb, 8);
+ VALIDATE_INDEX(floor_setup->data.t1.class_masterbook[j], vc->codebook_count)
AV_DEBUG(" masterbook: %d \n", floor_setup->data.t1.class_masterbook[j]);
}
for(k=0;k<(1<<floor_setup->data.t1.class_subclasses[j]);++k) {
- int16_t bits=get_bits(gb, 8)-1;
- if (bits!=-1 && bits>=vc->codebook_count) {
- av_log(vc->avccontext, AV_LOG_ERROR, "Subclass book index %d is out of range.\n", bits);
- return -1;
- }
- floor_setup->data.t1.subclass_books[j][k]=bits;
+ floor_setup->data.t1.subclass_books[j][k]=(int16_t)get_bits(gb, 8)-1;
+ if (floor_setup->data.t1.subclass_books[j][k]!=-1)
+ VALIDATE_INDEX(floor_setup->data.t1.subclass_books[j][k], vc->codebook_count)
AV_DEBUG(" book %d. : %d \n", k, floor_setup->data.t1.subclass_books[j][k]);
}
@@ -564,9 +566,8 @@
uint_fast8_t book_idx;
for (idx=0;idx<floor_setup->data.t0.num_books;++idx) {
book_idx=get_bits(gb, 8);
- if (book_idx>=vc->codebook_count)
- return -1;
floor_setup->data.t0.book_list[idx]=book_idx;
+ VALIDATE_INDEX(floor_setup->data.t0.book_list[idx], vc->codebook_count)
if (vc->codebooks[book_idx].dimensions > max_codebook_dim)
max_codebook_dim=vc->codebooks[book_idx].dimensions;
}
@@ -649,10 +650,7 @@
res_setup->classifications=get_bits(gb, 6)+1;
res_setup->classbook=get_bits(gb, 8);
- if (res_setup->classbook>=vc->codebook_count) {
- av_log(vc->avccontext, AV_LOG_ERROR, "classbook value %d out of range. \n", res_setup->classbook);
- return -1;
- }
+ VALIDATE_INDEX(res_setup->classbook, vc->codebook_count)
AV_DEBUG(" begin %d end %d part.size %d classif.s %d classbook %d \n", res_setup->begin, res_setup->end, res_setup->partition_size,
res_setup->classifications, res_setup->classbook);
@@ -672,12 +670,8 @@
for(j=0;j<res_setup->classifications;++j) {
for(k=0;k<8;++k) {
if (cascade[j]&(1<<k)) {
- int bits=get_bits(gb, 8);
- if (bits>=vc->codebook_count) {
- av_log(vc->avccontext, AV_LOG_ERROR, "book value %d out of range. \n", bits);
- return -1;
- }
- res_setup->books[j][k]=bits;
+ res_setup->books[j][k]=get_bits(gb, 8);
+ VALIDATE_INDEX(res_setup->books[j][k], vc->codebook_count)
AV_DEBUG(" %d class casscade depth %d book: %d \n", j, k, res_setup->books[j][k]);
@@ -724,14 +718,8 @@
for(j=0;j<mapping_setup->coupling_steps;++j) {
mapping_setup->magnitude[j]=get_bits(gb, ilog(vc->audio_channels-1));
mapping_setup->angle[j]=get_bits(gb, ilog(vc->audio_channels-1));
- if (mapping_setup->magnitude[j]>=vc->audio_channels) {
- av_log(vc->avccontext, AV_LOG_ERROR, "magnitude channel %d out of range. \n", mapping_setup->magnitude[j]);
- return -1;
- }
- if (mapping_setup->angle[j]>=vc->audio_channels) {
- av_log(vc->avccontext, AV_LOG_ERROR, "angle channel %d out of range. \n", mapping_setup->angle[j]);
- return -1;
- }
+ VALIDATE_INDEX(mapping_setup->magnitude[j], vc->audio_channels)
+ VALIDATE_INDEX(mapping_setup->angle [j], vc->audio_channels)
}
} else {
mapping_setup->coupling_steps=0;
@@ -752,20 +740,11 @@
}
for(j=0;j<mapping_setup->submaps;++j) {
- int bits;
skip_bits(gb, 8); // FIXME check?
- bits=get_bits(gb, 8);
- if (bits>=vc->floor_count) {
- av_log(vc->avccontext, AV_LOG_ERROR, "submap floor value %d out of range. \n", bits);
- return -1;
- }
- mapping_setup->submap_floor[j]=bits;
- bits=get_bits(gb, 8);
- if (bits>=vc->residue_count) {
- av_log(vc->avccontext, AV_LOG_ERROR, "submap residue value %d out of range. \n", bits);
- return -1;
- }
- mapping_setup->submap_residue[j]=bits;
+ mapping_setup->submap_floor[j]=get_bits(gb, 8);
+ VALIDATE_INDEX(mapping_setup->submap_floor[j], vc->floor_count)
+ mapping_setup->submap_residue[j]=get_bits(gb, 8);
+ VALIDATE_INDEX(mapping_setup->submap_residue[j], vc->residue_count)
AV_DEBUG(" %d mapping %d submap : floor %d, residue %d \n", i, j, mapping_setup->submap_floor[j], mapping_setup->submap_residue[j]);
}
@@ -829,10 +808,7 @@
mode_setup->windowtype=get_bits(gb, 16); //FIXME check
mode_setup->transformtype=get_bits(gb, 16); //FIXME check
mode_setup->mapping=get_bits(gb, 8);
- if (mode_setup->mapping>=vc->mapping_count) {
- av_log(vc->avccontext, AV_LOG_ERROR, "mode mapping value %d out of range. \n", mode_setup->mapping);
- return -1;
- }
+ VALIDATE_INDEX(mode_setup->mapping, vc->mapping_count)
AV_DEBUG(" %d mode: blockflag %d, windowtype %d, transformtype %d, mapping %d \n", i, mode_setup->blockflag, mode_setup->windowtype, mode_setup->transformtype, mode_setup->mapping);
}
@@ -1489,11 +1465,8 @@
} else {
mode_number=get_bits(gb, ilog(vc->mode_count-1));
}
- if (mode_number>=vc->mode_count) {
- av_log(vc->avccontext, AV_LOG_ERROR, "mode number %d out of range.\n", mode_number);
- return -1;
- }
vc->mode_number=mode_number;
+ VALIDATE_INDEX(vc->mode_number, vc->mode_count)
mapping=&vc->mappings[vc->modes[mode_number].mapping];
AV_DEBUG(" Mode number: %d , mapping: %d , blocktype %d \n", mode_number, vc->modes[mode_number].mapping, vc->modes[mode_number].blockflag);
More information about the ffmpeg-devel
mailing list