diff mbox series

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

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

Commit Message

Eric Wong July 15, 2024, 12:35 a.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.

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

Comments

Patrick Steinhardt July 24, 2024, 8:35 a.m. UTC | #1
On Mon, Jul 15, 2024 at 12:35:12AM +0000, Eric Wong wrote:
> 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
Eric Wong July 26, 2024, 7:43 a.m. UTC | #2
Patrick Steinhardt <ps@pks.im> wrote:
> 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.

Right, it's just for consistency with the rest of the code base.
Will update the message for v2.
diff mbox series

Patch

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)