Re: [PATCH 02/26] patch-id: convert to use the_hash_algo
- From
René Scharfe <l.s.r@web.de>
- Date
- Aug 20, 2019, 21:12 UTC
- Message-ID
- <1d3b46de-155a-6072-5e44-2622ac821230@web.de>
- In-Reply-To
- <20190818200427.870753-3-sandals@crustytoothpaste.net>
Am 18.08.19 um 22:04 schrieb brian m. carlson:
Show 22 quoted lines
> diff --git a/builtin/patch-id.c b/builtin/patch-id.c
> index bd28b80b2d..3059e525b8 100644
> --- a/builtin/patch-id.c
> +++ b/builtin/patch-id.c
> @@ -1,15 +1,16 @@
> +#include "cache.h"
> #include "builtin.h"
> #include "config.h"
> #include "diff.h"
>
> static void flush_current_id(int patchlen, struct object_id *id, struct object_id *result)
> {
> - char name[50];
> + char name[GIT_MAX_HEXSZ + 1];
>
> if (!patchlen)
> return;
>
> - memcpy(name, oid_to_hex(id), GIT_SHA1_HEXSZ + 1);
> + memcpy(name, oid_to_hex(id), the_hash_algo->hexsz + 1);
> printf("%s %s\n", oid_to_hex(result), name);
> }OK. But why do we need our own buffer? oid_to_hex() provides four of them for us, so the body could become just:
if (patchlen)
printf("%s %s\n", oid_to_hex(result), oid_to_hex(id));Right? Well, this buffer comes from f97672225b («Add "git-patch-id" program to generate patch ID's.», 2005-06-23), which predates the introduction of the four-buffer feature in dcb3450fd8 («sha1_to_hex() usage cleanup», 2006-05-03).
And with 30e12b924b («patch-id: make it stable against hunk reordering», 2014-04-27) the function's name became a bit misleading, because it stopped being responsible for flushing the hash calculation.
So perhaps this on top? (Or squash it in, if you like, but it's certainly not worth a re-roll.)
-- >8 -- Subject: [PATCH] patch-id: inline flush_current_id()
The function hasn't been flushing the hash calculation since 30e12b924b ("patch-id: make it stable against hunk reordering", 2014-04-27), and there is no need for a private copy of the second hexadecimal hash value since dcb3450fd8 ("sha1_to_hex() usage cleanup", 2006-05-03) added support for up to four sha1_to_hex() results to be used in the same printf(3) call, which oid_to_hex() inherited. So print both hash values directly and get rid of the function with the outdated name.
Signed-off-by: René Scharfe <l.s.r@web.de> --- builtin/patch-id.c | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-)
diff --git a/builtin/patch-id.c b/builtin/patch-id.c index 3059e525b8..d328714af7 100644 --- a/builtin/patch-id.c +++ b/builtin/patch-id.c @@ -3,17 +3,6 @@ #include "config.h" #include "diff.h" -static void flush_current_id(int patchlen, struct object_id *id, struct object_id *result) -{ - char name[GIT_MAX_HEXSZ + 1]; - - if (!patchlen) - return; - - memcpy(name, oid_to_hex(id), the_hash_algo->hexsz + 1); - printf("%s %s\n", oid_to_hex(result), name); -} - static int remove_space(char *line) { char *src = line; @@ -137,13 +126,12 @@ static int get_one_patchid(struct object_id *next_oid, struct object_id *result, static void generate_id_list(int stable) { struct object_id oid, n, result; - int patchlen; struct strbuf line_buf = STRBUF_INIT; oidclr(&oid); while (!feof(stdin)) { - patchlen = get_one_patchid(&n, &result, &line_buf, stable); - flush_current_id(patchlen, &oid, &result); + if (get_one_patchid(&n, &result, &line_buf, stable)) + printf("%s %s\n", oid_to_hex(&result), oid_to_hex(&oid)); oidcpy(&oid, &n); } strbuf_release(&line_buf); -- 2.23.0