Re: [PATCH v7 04/13] pack-objects: use bitfield for object_entry::depth

From: Jeff King <peff@peff.net>
Date: Mar 30, 2018, 20:26 UTC
Message-ID: <20180330202645.GC14782@sigill.intra.peff.net>
In-Reply-To: <20180324063353.24722-5-pclouds@gmail.com>

On Sat, Mar 24, 2018 at 07:33:44AM +0100, Nguyễn Thái Ngọc Duy wrote:

> Because of struct packing from now on we can only handle max depth
> 4095 (or even lower when new booleans are added in this struct). This
> should be ok since long delta chain will cause significant slow down
> anyway.

OK. This is the first user-facing change, but I think it really shouldn't hurt anybody. My experiments a while ago showed that chains longer than 50 aren't really worth it, but so this could probably shrink to something like 8 bits if we really needed it to.

Show 11 quoted lines

> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index 83f8154865..205e1f646c 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -3068,6 +3068,10 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
>  	if (pack_to_stdout != !base_name || argc)
>  		usage_with_options(pack_usage, pack_objects_options);
>  
> +	if (depth >= (1 << OE_DEPTH_BITS))
> +		die(_("delta chain depth %d is greater than maximum limit %d"),
> +		    depth, (1 << OE_DEPTH_BITS) - 1);

Since this is introducing a new limit, I wonder if we should issue a warning and just clamp it to the maximum value. That would be kinder to people who may have existing (admittedly dumb) setups.

-Peff