[FFmpeg-devel] [RFC] consistify/simplify/whatever index validation in vorbis_dec

[Michael Niedermayer]

On Tue, Oct 27, 2009 at 03:49:40PM +0100, Reimar D?ffinger wrote:
> On Mon, Oct 05, 2009 at 09:18:13PM +0200, Michael Niedermayer wrote:
> > > ===================================================================
> > > --- 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;\
> > > +    }
> > 
> > get_bits() can be included in the macro
> 
> In most cases. Somewhat improved version attached.

>  vorbis_dec.c |   86 +++++++++++++++++++----------------------------------------
>  1 file changed, 28 insertions(+), 58 deletions(-)
> 13e981c468e227c1344ac7bb1bb065af3de5ed9e  vorbisvalidate.diff
> Index: libavcodec/vorbis_dec.c
> ===================================================================
> --- libavcodec/vorbis_dec.c	(revision 20376)
> +++ libavcodec/vorbis_dec.c	(working copy)
> @@ -160,6 +160,22 @@
>  #define BARK(x) \
>      (13.1f * atan(0.00074f * (x)) + 2.24f * atan(1.85e-8f * (x) * (x)) + 1e-4f * (x))
>  
> +static const char idx_err_str[] = "Index value %d out of range (0 - %d) for %s at %s:%i\n";
> +#define VALIDATE_INDEX(idx, limit) \
> +    if (idx >= limit) {\
> +        av_log(vc->avccontext, AV_LOG_ERROR,\
> +               idx_err_str,\
> +               (int)(idx), (int)(limit - 1), #idx, __FILE__, __LINE__);\
> +        return -1;\
> +    }
> +#define GET_VALIDATED_INDEX(idx, bits, limit) \
> +    {\
> +        idx = get_bits(gb, bits);\
> +        VALIDATE_INDEX(idx, limit)\
> +    }

> +#define GET_CODEBOOK_INDEX(idx) \
> +    GET_VALIDATED_INDEX(idx, 8, vc->codebook_count)
> +

i think this one just makes the code harder to read (what was 
GET_CODEBOOK_INDEX, where is that ...)

except that, iam ok with it

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

It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20091027/3ded6534/attachment.pgp>