Re: [PATCH v1 03/10] packfile: fix off-by-one in content_limit comparison

From: Patrick Steinhardt <ps@pks.im>
Date: Jul 24, 2024, 08:35 UTC
Message-ID: <ZqC86t7YpVhdmLh_@tanuki>
In-Reply-To: <20240715003519.2671385-4-e@80x24.org>

On Mon, Jul 15, 2024 at 12:35:12AM +0000, Eric Wong wrote:

Show 23 quoted lines

> object-file.c::loose_object_info() accepts objects matching
> content_limit exactly, so it follows packfile handling allows
> slurping objects which match loose object handling and slurp
> objects with size matching the content_limit exactly.
> 
> Signed-off-by: Eric Wong <e@80x24.org>
> ---
>  packfile.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/packfile.c b/packfile.c
> index 54b9d46928..371da96cdb 100644
> --- a/packfile.c
> +++ b/packfile.c
> @@ -1558,7 +1558,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,
>  		}
>  
>  		if (oi->contentp) {
> -			if (oi->sizep && *oi->sizep < oi->content_limit) {
> +			if (oi->sizep && *oi->sizep <= oi->content_limit) {
>  				*oi->contentp = cache_or_unpack_entry(r, p, obj_offset,
>  								      oi->sizep, &type);
>  				if (!*oi->contentp)

In practice this doesn't really fix a user-visible bug, right? The only difference before and after is that we now start to stream contents earlier? And that's why we cannot have a test for this.

If so, I'd recommend to explain this in the commit message.

Patrick