diff mbox series

[v2,03/10] packfile: fix off-by-one in content_limit comparison

Message ID 20240823224630.1180772-4-e@80x24.org (mailing list archive)
State New
Headers show
Series cat-file speedups | expand

Commit Message

Eric Wong Aug. 23, 2024, 10:46 p.m. UTC
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.

This change is merely for consistency with the majority of
existing code and there is no user visible change in nearly all
cases.  The only exception being the corner case when the object
size matches content_limit exactly where users will see a
speedup from avoiding an extra lookup.

Signed-off-by: Eric Wong <e@80x24.org>
---
 packfile.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Junio C Hamano Aug. 26, 2024, 4:55 p.m. UTC | #1
Eric Wong <e@80x24.org> writes:

> 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.
>
> This change is merely for consistency with the majority of
> existing code and there is no user visible change in nearly all
> cases.  The only exception being the corner case when the object
> size matches content_limit exactly where users will see a
> speedup from avoiding an extra lookup.
>
> Signed-off-by: Eric Wong <e@80x24.org>
> ---

I would have preferred to see this (and also "is oi->content_limit
zero?" check I mentioned earlier) as part of the previous step,
which added this comparison that is not consistent with the majority
of existing code.  It's not like importing from an external project
we communicate with only occasionally, in which case we may want to
import "pristine" source and fix it up separetly in order to make it
easier to re-import updated material.

>  packfile.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/packfile.c b/packfile.c
> index c12a0515b3..8ec86d2d69 100644
> --- a/packfile.c
> +++ b/packfile.c
> @@ -1557,7 +1557,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)
diff mbox series

Patch

diff --git a/packfile.c b/packfile.c
index c12a0515b3..8ec86d2d69 100644
--- a/packfile.c
+++ b/packfile.c
@@ -1557,7 +1557,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)