Subject: Re: [PATCH v3 7/8] odb: guard against data loss checking out a huge file

From: Junio C Hamano <[email-masked]>
Date: Fri, 29 Oct 2021 16:13:37 -0700

"Matt Cooper via GitGitGadget" <[email-masked]> writes:

> diff --git a/delta.h b/delta.h
> index 2df5fe13d95..8a56ec07992 100644
> --- a/delta.h
> +++ b/delta.h
> @@ -90,15 +90,15 @@ static inline unsigned long get_delta_hdr_size(const unsigned char **datap,
>  					       const unsigned char *top)
>  {
>  	const unsigned char *data = *datap;
> -	unsigned long cmd, size = 0;
> +	size_t cmd, size = 0;
>  	int i = 0;
>  	do {
>  		cmd = *data++;
> -		size |= (cmd & 0x7f) << i;
> +		size |= st_left_shift(cmd & 0x7f, i);
>  		i += 7;
>  	} while (cmd & 0x80 && data < top);
>  	*datap = data;
> -	return size;
> +	return cast_size_t_to_ulong(size);
>  }

OK.  The patch-delta code is reasonably self contained, so the next
step to measure the in-core object size in size_t shouldn't be too
bad, but before everything got size_t aware, we have to have the
"safe casting" somewhere, and I agree this "immediately before
return" is a good place to draw the line.

Nicely done.

>  			if (c > 9)
>  				break;
>  			hdr++;
> -			size = size * 10 + c;
> +			size = st_add(st_mult(size, 10), c);

Nice.

>  	if (oi->sizep)
> -		*oi->sizep = size;
> +		*oi->sizep = cast_size_t_to_ulong(size);

OK.

> @@ -1073,10 +1073,10 @@ unsigned long unpack_object_header_buffer(const unsigned char *buf,
>  			break;
>  		}
>  		c = buf[used++];
> -		size += (c & 0x7f) << shift;
> +		size = st_add(size, st_left_shift(c & 0x7f, shift));
>  		shift += 7;
>  	}
> -	*sizep = size;
> +	*sizep = cast_size_t_to_ulong(size);

OK.

Looking good.