[PATCH] pack-objects: fix comment of reused_chunk.difference

Han Xin· Mar 23, 2021, 03:20 UTC · lore

From: Han Xin <hanxin.hx@alibaba-inc.com>

As record_reused_object(offset, offset - hashfile_total(out)) said, reused_chunk.difference should be the offset of original packfile minus the offset of the generated packfile. But the comment presented an opposite way.

Signed-off-by: Han Xin <hanxin.hx@alibaba-inc.com>
---
 builtin/pack-objects.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Show changes to builtin/pack-objects.c +2 −2

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 5617c01b5a..0c45bdc011 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -810,8 +810,8 @@ static struct reused_chunk {
 	/* The offset of the first object of this chunk in the original
 	 * packfile. */
 	off_t original;
-	/* The offset of the first object of this chunk in the generated
-	 * packfile minus "original". */
+	/* The difference for "original" minus the offset of the first object of
+	 * this chunk in the generated packfile. */
 	off_t difference;
 } *reused_chunks;
 static int reused_chunks_nr;

-- 
2.24.1

Jeff King· Mar 24, 2021, 18:43 UTC · re: Han Xin · lore

Re: [PATCH] pack-objects: fix comment of reused_chunk.difference

On Tue, Mar 23, 2021 at 11:20:50AM +0800, Han Xin wrote:

Show 16 quoted lines

> From: Han Xin <hanxin.hx@alibaba-inc.com>
> 
> As record_reused_object(offset, offset - hashfile_total(out)) said,
> reused_chunk.difference should be the offset of original packfile minus
> the offset of the generated packfile. But the comment presented an opposite way.
> [...]
>
> @@ -810,8 +810,8 @@ static struct reused_chunk {
>  	/* The offset of the first object of this chunk in the original
>  	 * packfile. */
>  	off_t original;
> -	/* The offset of the first object of this chunk in the generated
> -	 * packfile minus "original". */
> +	/* The difference for "original" minus the offset of the first object of
> +	 * this chunk in the generated packfile. */
>  	off_t difference;

The naming and comments here came from Christian's upstreaming of the topic (cc'd). In the original, this was just called "offset". And there was no comment, so it couldn't be wrong. ;)

I think your suggestion here is correct. The value comes from the "offset" parameter of record_reused_object(), and we pass:

  record_reused_object(offset, offset - hashfile_total(out));

(where "offset" here is the pack position in the original packfile). So it is definitely "original_offset - output_offset".

All of which is a long-winded way of saying "looks good to me". :)

-Peff

patchpack-objects: fix comment of reused_chunk.difference