Re: [PATCH 3/7] inflate pt1: clean up input logic

[Matt Mackall <mpm@selenic.com>]

On Fri, Feb 24, 2006 at 10:19:09PM +0000, Russell King wrote:
> On Fri, Feb 24, 2006 at 02:12:16PM -0600, Matt Mackall wrote:
> > -static const u16 mask_bits[] = {
> > -	0x0000,
> > -	0x0001, 0x0003, 0x0007, 0x000f, 0x001f, 0x003f, 0x007f, 0x00ff,
> > -	0x01ff, 0x03ff, 0x07ff, 0x0fff, 0x1fff, 0x3fff, 0x7fff, 0xffff
> > -};
> > +static inline u32 readbits(u32 *b, u32 *k, int n)
> > +{
> > +	for( ; *k < n; *k += 8)
> > +		*b |= (u32)get_byte() << *k;
> > +	return *b & ((1 << n) - 1);
> > +}
> >  
> > -#define NEXTBYTE()  ({ int v = get_byte(); if (v < 0) goto underrun; (u8)v; })
> 
> How does this change handle the case where we run out of input data?
> This condition needs to be handled explicitly because the inflate
> functions can infinitely loop.

And if you look at the current users, you'll see that only two of 15
actually use it.

> Relying on a bit pattern returned by get_byte() is how this code
> pre-fix used to work, and it caused several confused bug reports.

Just about everywhere, get_byte prints an error message and halts. In
the final refactoring, get_byte goes away and is replaced by a
->fill() method that's only called when the input buffer is emptied,
rather than byte by byte. Most of the users leave this null (since
they have the entire contents in memory already), which will give us a
nice oops. I can add another patch in the next batch that makes an
attempt to call a null ->fill instead do ->error("gunzip underrun").

-- 
Mathematics is the supreme nostalgia of our time.