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

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

[Han Xin]

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(-)

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

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

[Jeff King]

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

> 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