{"thread":{"id":"55367","subject":"[PATCH] pack-objects: fix comment of reused_chunk.difference","startedAt":"2021-03-23T03:22:22Z","lastAt":"2021-03-24T18:44:16Z","messageCount":2,"participants":["Han Xin","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"419980","messageId":"20210323032050.97700-1-chiyutianyi@gmail.com","threadId":"55367","inReplyTo":null,"subject":"[PATCH] pack-objects: fix comment of reused_chunk.difference","fromName":"Han Xin","fromEmail":"chiyutianyi@gmail.com","sentAt":"2021-03-23T03:20:50Z","receivedAt":"2021-03-23T03:22:22Z","isPatch":true,"sender":{"key":"chiyutianyi@gmail.com","avatar":null},"body":"From: Han Xin <hanxin.hx@alibaba-inc.com>\n\nAs record_reused_object(offset, offset - hashfile_total(out)) said,\nreused_chunk.difference should be the offset of original packfile minus\nthe offset of the generated packfile. But the comment presented an opposite way.\n\nSigned-off-by: Han Xin <hanxin.hx@alibaba-inc.com>\n---\n builtin/pack-objects.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 5617c01b5a..0c45bdc011 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -810,8 +810,8 @@ static struct reused_chunk {\n \t/* The offset of the first object of this chunk in the original\n \t * packfile. */\n \toff_t original;\n-\t/* The offset of the first object of this chunk in the generated\n-\t * packfile minus \"original\". */\n+\t/* The difference for \"original\" minus the offset of the first object of\n+\t * this chunk in the generated packfile. */\n \toff_t difference;\n } *reused_chunks;\n static int reused_chunks_nr;\n-- \n2.24.1\n\n"},{"id":"420127","messageId":"YFuIXvQSUzbJgDLp@coredump.intra.peff.net","threadId":"55367","inReplyTo":"20210323032050.97700-1-chiyutianyi@gmail.com","subject":"Re: [PATCH] pack-objects: fix comment of reused_chunk.difference","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-03-24T18:43:42Z","receivedAt":"2021-03-24T18:44:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 23, 2021 at 11:20:50AM +0800, Han Xin wrote:\n\n> From: Han Xin <hanxin.hx@alibaba-inc.com>\n> \n> As record_reused_object(offset, offset - hashfile_total(out)) said,\n> reused_chunk.difference should be the offset of original packfile minus\n> the offset of the generated packfile. But the comment presented an opposite way.\n> [...]\n>\n> @@ -810,8 +810,8 @@ static struct reused_chunk {\n>  \t/* The offset of the first object of this chunk in the original\n>  \t * packfile. */\n>  \toff_t original;\n> -\t/* The offset of the first object of this chunk in the generated\n> -\t * packfile minus \"original\". */\n> +\t/* The difference for \"original\" minus the offset of the first object of\n> +\t * this chunk in the generated packfile. */\n>  \toff_t difference;\n\nThe naming and comments here came from Christian's upstreaming of the\ntopic (cc'd). In the original, this was just called \"offset\". And there\nwas no comment, so it couldn't be wrong. ;)\n\nI think your suggestion here is correct. The value comes from the\n\"offset\" parameter of record_reused_object(), and we pass:\n\n  record_reused_object(offset, offset - hashfile_total(out));\n\n(where \"offset\" here is the pack position in the original packfile). So\nit is definitely \"original_offset - output_offset\".\n\nAll of which is a long-winded way of saying \"looks good to me\". :)\n\n-Peff\n"}]}