diff mbox series

[02/26] patch-id: convert to use the_hash_algo

Message ID 20190818200427.870753-3-sandals@crustytoothpaste.net (mailing list archive)
State New, archived
Headers show
Series object_id part 17 | expand

Commit Message

brian m. carlson Aug. 18, 2019, 8:04 p.m. UTC 
Convert the two separate patch-id implementations to use the_hash_algo
in their implementation.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/patch-id.c | 11 ++++++-----
 diff.c             | 46 +++++++++++++++++++++++-----------------------
 diff.h             |  2 +-
 3 files changed, 30 insertions(+), 29 deletions(-)

Comments

René Scharfe Aug. 20, 2019, 9:12 p.m. UTC | #1 
Am 18.08.19 um 22:04 schrieb brian m. carlson:
> 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
brian m. carlson Aug. 20, 2019, 10:36 p.m. UTC | #2 
On 2019-08-20 at 21:12:00, René Scharfe wrote:
> So perhaps this on top?  (Or squash it in, if you like, but it's
> certainly not worth a re-roll.)

Yeah, this seems sensible.  I'll include it if I do a re-roll.
Junio C Hamano Aug. 22, 2019, 3:53 p.m. UTC | #3 
"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> On 2019-08-20 at 21:12:00, René Scharfe wrote:
>> So perhaps this on top?  (Or squash it in, if you like, but it's
>> certainly not worth a re-roll.)
>
> Yeah, this seems sensible.  I'll include it if I do a re-roll.

Thanks.
diff mbox series

Patch

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);
 }
 
@@ -60,9 +61,9 @@  static int get_one_patchid(struct object_id *next_oid, struct object_id *result,
 {
 	int patchlen = 0, found_next = 0;
 	int before = -1, after = -1;
-	git_SHA_CTX ctx;
+	git_hash_ctx ctx;
 
-	git_SHA1_Init(&ctx);
+	the_hash_algo->init_fn(&ctx);
 	oidclr(result);
 
 	while (strbuf_getwholeline(line_buf, stdin, '\n') != EOF) {
@@ -122,7 +123,7 @@  static int get_one_patchid(struct object_id *next_oid, struct object_id *result,
 		/* Compute the sha without whitespace */
 		len = remove_space(line);
 		patchlen += len;
-		git_SHA1_Update(&ctx, line, len);
+		the_hash_algo->update_fn(&ctx, line, len);
 	}
 
 	if (!found_next)
diff --git a/diff.c b/diff.c
index efe42b341a..bf51c94804 100644
--- a/diff.c
+++ b/diff.c
@@ -5978,7 +5978,7 @@  static void diff_summary(struct diff_options *opt, struct diff_filepair *p)
 }
 
 struct patch_id_t {
-	git_SHA_CTX *ctx;
+	git_hash_ctx *ctx;
 	int patchlen;
 };
 
@@ -5995,16 +5995,16 @@  static int remove_space(char *line, int len)
 	return dst - line;
 }
 
-void flush_one_hunk(struct object_id *result, git_SHA_CTX *ctx)
+void flush_one_hunk(struct object_id *result, git_hash_ctx *ctx)
 {
 	unsigned char hash[GIT_MAX_RAWSZ];
 	unsigned short carry = 0;
 	int i;
 
-	git_SHA1_Final(hash, ctx);
-	git_SHA1_Init(ctx);
+	the_hash_algo->final_fn(hash, ctx);
+	the_hash_algo->init_fn(ctx);
 	/* 20-byte sum, with carry */
-	for (i = 0; i < GIT_SHA1_RAWSZ; ++i) {
+	for (i = 0; i < the_hash_algo->rawsz; ++i) {
 		carry += result->hash[i] + hash[i];
 		result->hash[i] = carry;
 		carry >>= 8;
@@ -6018,21 +6018,21 @@  static void patch_id_consume(void *priv, char *line, unsigned long len)
 
 	new_len = remove_space(line, len);
 
-	git_SHA1_Update(data->ctx, line, new_len);
+	the_hash_algo->update_fn(data->ctx, line, new_len);
 	data->patchlen += new_len;
 }
 
-static void patch_id_add_string(git_SHA_CTX *ctx, const char *str)
+static void patch_id_add_string(git_hash_ctx *ctx, const char *str)
 {
-	git_SHA1_Update(ctx, str, strlen(str));
+	the_hash_algo->update_fn(ctx, str, strlen(str));
 }
 
-static void patch_id_add_mode(git_SHA_CTX *ctx, unsigned mode)
+static void patch_id_add_mode(git_hash_ctx *ctx, unsigned mode)
 {
 	/* large enough for 2^32 in octal */
 	char buf[12];
 	int len = xsnprintf(buf, sizeof(buf), "%06o", mode);
-	git_SHA1_Update(ctx, buf, len);
+	the_hash_algo->update_fn(ctx, buf, len);
 }
 
 /* returns 0 upon success, and writes result into oid */
@@ -6040,10 +6040,10 @@  static int diff_get_patch_id(struct diff_options *options, struct object_id *oid
 {
 	struct diff_queue_struct *q = &diff_queued_diff;
 	int i;
-	git_SHA_CTX ctx;
+	git_hash_ctx ctx;
 	struct patch_id_t data;
 
-	git_SHA1_Init(&ctx);
+	the_hash_algo->init_fn(&ctx);
 	memset(&data, 0, sizeof(struct patch_id_t));
 	data.ctx = &ctx;
 	oidclr(oid);
@@ -6076,27 +6076,27 @@  static int diff_get_patch_id(struct diff_options *options, struct object_id *oid
 		len2 = remove_space(p->two->path, strlen(p->two->path));
 		patch_id_add_string(&ctx, "diff--git");
 		patch_id_add_string(&ctx, "a/");
-		git_SHA1_Update(&ctx, p->one->path, len1);
+		the_hash_algo->update_fn(&ctx, p->one->path, len1);
 		patch_id_add_string(&ctx, "b/");
-		git_SHA1_Update(&ctx, p->two->path, len2);
+		the_hash_algo->update_fn(&ctx, p->two->path, len2);
 
 		if (p->one->mode == 0) {
 			patch_id_add_string(&ctx, "newfilemode");
 			patch_id_add_mode(&ctx, p->two->mode);
 			patch_id_add_string(&ctx, "---/dev/null");
 			patch_id_add_string(&ctx, "+++b/");
-			git_SHA1_Update(&ctx, p->two->path, len2);
+			the_hash_algo->update_fn(&ctx, p->two->path, len2);
 		} else if (p->two->mode == 0) {
 			patch_id_add_string(&ctx, "deletedfilemode");
 			patch_id_add_mode(&ctx, p->one->mode);
 			patch_id_add_string(&ctx, "---a/");
-			git_SHA1_Update(&ctx, p->one->path, len1);
+			the_hash_algo->update_fn(&ctx, p->one->path, len1);
 			patch_id_add_string(&ctx, "+++/dev/null");
 		} else {
 			patch_id_add_string(&ctx, "---a/");
-			git_SHA1_Update(&ctx, p->one->path, len1);
+			the_hash_algo->update_fn(&ctx, p->one->path, len1);
 			patch_id_add_string(&ctx, "+++b/");
-			git_SHA1_Update(&ctx, p->two->path, len2);
+			the_hash_algo->update_fn(&ctx, p->two->path, len2);
 		}
 
 		if (diff_header_only)
@@ -6108,10 +6108,10 @@  static int diff_get_patch_id(struct diff_options *options, struct object_id *oid
 
 		if (diff_filespec_is_binary(options->repo, p->one) ||
 		    diff_filespec_is_binary(options->repo, p->two)) {
-			git_SHA1_Update(&ctx, oid_to_hex(&p->one->oid),
-					GIT_SHA1_HEXSZ);
-			git_SHA1_Update(&ctx, oid_to_hex(&p->two->oid),
-					GIT_SHA1_HEXSZ);
+			the_hash_algo->update_fn(&ctx, oid_to_hex(&p->one->oid),
+					the_hash_algo->hexsz);
+			the_hash_algo->update_fn(&ctx, oid_to_hex(&p->two->oid),
+					the_hash_algo->hexsz);
 			continue;
 		}
 
@@ -6128,7 +6128,7 @@  static int diff_get_patch_id(struct diff_options *options, struct object_id *oid
 	}
 
 	if (!stable)
-		git_SHA1_Final(oid->hash, &ctx);
+		the_hash_algo->final_fn(oid->hash, &ctx);
 
 	return 0;
 }
diff --git a/diff.h b/diff.h
index c2c3056810..7f8f024feb 100644
--- a/diff.h
+++ b/diff.h
@@ -438,7 +438,7 @@  int run_diff_index(struct rev_info *revs, int cached);
 
 int do_diff_cache(const struct object_id *, struct diff_options *);
 int diff_flush_patch_id(struct diff_options *, struct object_id *, int, int);
-void flush_one_hunk(struct object_id *, git_SHA_CTX *);
+void flush_one_hunk(struct object_id *result, git_hash_ctx *ctx);
 
 int diff_result_code(struct diff_options *, int);