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

Subsystems: the rest

STALE2021d

2 messages, 2 authors, 2021-03-24 · open the first message on its own page

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

From: Han Xin <hidden>
Date: 2021-03-23 03:22:22

From: Han Xin <redacted>

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 <redacted>
---
 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

From: Jeff King <hidden>
Date: 2021-03-24 18:44:16

On Tue, Mar 23, 2021 at 11:20:50AM +0800, Han Xin wrote:
quoted hunk
From: Han Xin <redacted>

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help