[FFmpeg-cvslog] alsdec: check block length

[Michael Niedermayer]

On Wed, Dec 12, 2012 at 03:34:05PM +0100, Thilo Borgmann wrote:
> Some nits follow...
> 
> 
> Am 12.12.12 14:20, schrieb Michael Niedermayer:
> > ffmpeg | branch: master | Michael Niedermayer <michaelni at gmx.at> | Wed Dec 12 12:28:45 2012 +0100| [0ceca269b66ec12a23bf0907bd2c220513cdbf16] | committer: Michael Niedermayer
> > 
> > alsdec: check block length
> > 
> > Fix writing over the end
> 
> Writing where over what end in which case?

block_length = 0
...
int      smp = bd->block_length - 1;
...
for (; smp; smp--)
        *dst++ = val;


> 
> > [...]
> >  /** Read the block data for a constant block
> >   */
> > -static void read_const_block_data(ALSDecContext *ctx, ALSBlockData *bd)
> > +static int read_const_block_data(ALSDecContext *ctx, ALSBlockData *bd)
> >  {
> >      ALSSpecificConfig *sconf = &ctx->sconf;
> >      AVCodecContext *avctx    = ctx->avctx;
> >      GetBitContext *gb        = &ctx->gb;
> >  
> > +    if (bd->block_length <= 0)
> > +        return -1;
> > +
> 
> Isn't there a proper AVERROR_ return value defined?
> Yes, this is not consistently done within the decoder. However, this is a new
> return value...

sure, i just did what the surrounding code does,
ill post a patch that fixes it in a moment


> 
> >      *bd->raw_samples = 0;
> >      *bd->const_block = get_bits1(gb);    // 1 = constant value, 0 = zero block (silence)
> >      bd->js_blocks    = get_bits1(gb);
> > @@ -573,6 +576,8 @@ static void read_const_block_data(ALSDecContext *ctx, ALSBlockData *bd)
> >  
> >      // ensure constant block decoding by reusing this field
> >      *bd->const_block = 1;
> > +
> > +    return 0;
> >  }
> >  
> >  
> > @@ -972,7 +977,8 @@ static int read_block(ALSDecContext *ctx, ALSBlockData *bd)
> 
> >          if (read_var_block_data(ctx, bd))
> >              return -1;
> >      } else {
> > -        read_const_block_data(ctx, bd);
> 
> > +        if (read_const_block_data(ctx, bd) < 0)
> 
> I would prefer it to be consistent with the handling of read_var_block_data()...
> so either " < 0 " for both or for none.

will post patch


> 
> 
> 
> Also I don't know if I've already seen this patch some time ago... then it is ok
> to find it in Log but otherwise I would prefer to find something new in Devel.

sorry thats my mistake, it felt trivial, i will post als patches to
devel in the future.

thanks

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

Democracy is the form of government in which you can choose your dictator
-------------- 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-cvslog/attachments/20121213/0a2bbc2e/attachment.asc>