Re: [PATCH] assorted delta code cleanup

[Matthias Urlichs <smurf@smurf.noris.de>]

Hi, Nicolas Pitre wrote:

> This is a wrap-up patch including all the cleanups I've done to the
> delta code and its usage. 

FWIW, I think Linus said he'd prefer a do/while loop. In fact this old
code ...

> -static unsigned long parse_delta_size(unsigned char **p)
> -{
> -	unsigned char c;
> -	unsigned long size = 0;
> -	unsigned shift = 0;
> -	unsigned char *data = *p;
> -
> -	do {
> -		c = *data++;
> -		size += (c & 0x7f) << shift;
> -		shift += 7;
> -	} while (c & 0x80);
> -	*p = data;
> -	return size;
> -}
> -

... is IMHO much more readable than this ...

> +static inline unsigned long get_delta_hdr_size(const unsigned char **datap)
> +{
> +	const unsigned char *data = *datap;
> +	unsigned char cmd = *data++;
> +	unsigned long size = cmd & ~0x80;
> +	int i = 7;
> +	while (cmd & 0x80) {
> +		cmd = *data++;
> +		size |= (cmd & ~0x80) << i;
> +		i += 7;
> +	}
> +	*datap = data;
> +	return size;
> +}
> +

(except that the first += should be |= ).

Yeah, I know, it's nitpicking. Regard it as a testimony to the code's
quality that I didn't see worse. ;-)

NB, it might be a good idea to exit with an error if the shift exceeds 31.

-- 
Matthias Urlichs   |   {M:U} IT Design @ m-u-it.de   |  smurf@smurf.noris.de
Disclaimer: The quote was selected randomly. Really. | http://smurf.noris.de
 - -
When it's fall in New York, the air smells as if someone's been frying
goats in it, and if you are keen to breathe the best plan is to open a
window and stick your head in a building.
		-- The Hitch Hiker's Guide to the Galaxy