[PATCH v2 02/24] hash: implement and use a context cloning function

From: brian m. carlson <sandals@crustytoothpaste.net>
Date: Feb 22, 2020, 20:17 UTC
Message-ID: <20200222201749.937983-3-sandals@crustytoothpaste.net>
In-Reply-To: <20200222201749.937983-1-sandals@crustytoothpaste.net>

For all of our SHA-1 implementations and most of our SHA-256 implementations, the hash context we use is a real struct. For these implementations, it's possible to copy a hash context by making a copy of the struct.

However, for our libgcrypt implementation, our hash context is a pointer. Consequently, copying it does not lead to an independent hash context like we intended.

Fortunately, however, libgcrypt provides us with a handy function to copy hash contexts. Let's add a cloning function to the hash algorithm API, and use it in the one place we need to make a hash context copy. With this change, our libgcrypt SHA-256 implementation is fully functional with all of our other hash implementations.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 csum-file.c     |  2 +-
 hash.h          | 21 +++++++++++++++++++++
 sha1-file.c     | 18 ++++++++++++++++++
 sha256/gcrypt.h |  6 ++++++
 4 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/csum-file.c b/csum-file.c
index 53ce37f7ca..0f35fa5ee4 100644
--- a/csum-file.c
+++ b/csum-file.c
@@ -157,7 +157,7 @@ void hashfile_checkpoint(struct hashfile *f, struct hashfile_checkpoint *checkpo
 {
 	hashflush(f);
 	checkpoint->offset = f->total;
-	checkpoint->ctx = f->ctx;
+	the_hash_algo->clone_fn(&checkpoint->ctx, &f->ctx);
 }
 
 int hashfile_truncate(struct hashfile *f, struct hashfile_checkpoint *checkpoint)
diff --git a/hash.h b/hash.h
index 52a4f1a3f4..e0f3f16b06 100644
--- a/hash.h
+++ b/hash.h
@@ -16,6 +16,7 @@
 #endif
 
 #if defined(SHA256_GCRYPT)
+#define SHA256_NEEDS_CLONE_HELPER
 #include "sha256/gcrypt.h"
 #elif defined(SHA256_OPENSSL)
 #include <openssl/sha.h>
@@ -54,12 +55,28 @@
 #define git_SHA256_Update	platform_SHA256_Update
 #define git_SHA256_Final	platform_SHA256_Final
 
+#ifdef platform_SHA256_Clone
+#define git_SHA256_Clone	platform_SHA256_Clone
+#endif
+
 #ifdef SHA1_MAX_BLOCK_SIZE
 #include "compat/sha1-chunked.h"
 #undef git_SHA1_Update
 #define git_SHA1_Update		git_SHA1_Update_Chunked
 #endif
 
+static inline void git_SHA1_Clone(git_SHA_CTX *dst, const git_SHA_CTX *src)
+{
+	memcpy(dst, src, sizeof(*dst));
+}
+
+#ifndef SHA256_NEEDS_CLONE_HELPER
+static inline void git_SHA256_Clone(git_SHA256_CTX *dst, const git_SHA256_CTX *src)
+{
+	memcpy(dst, src, sizeof(*dst));
+}
+#endif
+
 /*
  * Note that these constants are suitable for indexing the hash_algos array and
  * comparing against each other, but are otherwise arbitrary, so they should not
@@ -85,6 +102,7 @@ union git_hash_ctx {
 typedef union git_hash_ctx git_hash_ctx;
 
 typedef void (*git_hash_init_fn)(git_hash_ctx *ctx);
+typedef void (*git_hash_clone_fn)(git_hash_ctx *dst, const git_hash_ctx *src);
 typedef void (*git_hash_update_fn)(git_hash_ctx *ctx, const void *in, size_t len);
 typedef void (*git_hash_final_fn)(unsigned char *hash, git_hash_ctx *ctx);
 
@@ -110,6 +128,9 @@ struct git_hash_algo {
 	/* The hash initialization function. */
 	git_hash_init_fn init_fn;
 
+	/* The hash context cloning function. */
+	git_hash_clone_fn clone_fn;
+
 	/* The hash update function. */
 	git_hash_update_fn update_fn;
 
diff --git a/sha1-file.c b/sha1-file.c
index d785de8a85..c36ca5a545 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -74,6 +74,11 @@ static void git_hash_sha1_init(git_hash_ctx *ctx)
 	git_SHA1_Init(&ctx->sha1);
 }
 
+static void git_hash_sha1_clone(git_hash_ctx *dst, const git_hash_ctx *src)
+{
+	git_SHA1_Clone(&dst->sha1, &src->sha1);
+}
+
 static void git_hash_sha1_update(git_hash_ctx *ctx, const void *data, size_t len)
 {
 	git_SHA1_Update(&ctx->sha1, data, len);
@@ -90,6 +95,11 @@ static void git_hash_sha256_init(git_hash_ctx *ctx)
 	git_SHA256_Init(&ctx->sha256);
 }
 
+static void git_hash_sha256_clone(git_hash_ctx *dst, const git_hash_ctx *src)
+{
+	git_SHA256_Clone(&dst->sha256, &src->sha256);
+}
+
 static void git_hash_sha256_update(git_hash_ctx *ctx, const void *data, size_t len)
 {
 	git_SHA256_Update(&ctx->sha256, data, len);
@@ -105,6 +115,11 @@ static void git_hash_unknown_init(git_hash_ctx *ctx)
 	BUG("trying to init unknown hash");
 }
 
+static void git_hash_unknown_clone(git_hash_ctx *dst, const git_hash_ctx *src)
+{
+	BUG("trying to clone unknown hash");
+}
+
 static void git_hash_unknown_update(git_hash_ctx *ctx, const void *data, size_t len)
 {
 	BUG("trying to update unknown hash");
@@ -123,6 +138,7 @@ const struct git_hash_algo hash_algos[GIT_HASH_NALGOS] = {
 		0,
 		0,
 		git_hash_unknown_init,
+		git_hash_unknown_clone,
 		git_hash_unknown_update,
 		git_hash_unknown_final,
 		NULL,
@@ -136,6 +152,7 @@ const struct git_hash_algo hash_algos[GIT_HASH_NALGOS] = {
 		GIT_SHA1_HEXSZ,
 		GIT_SHA1_BLKSZ,
 		git_hash_sha1_init,
+		git_hash_sha1_clone,
 		git_hash_sha1_update,
 		git_hash_sha1_final,
 		&empty_tree_oid,
@@ -149,6 +166,7 @@ const struct git_hash_algo hash_algos[GIT_HASH_NALGOS] = {
 		GIT_SHA256_HEXSZ,
 		GIT_SHA256_BLKSZ,
 		git_hash_sha256_init,
+		git_hash_sha256_clone,
 		git_hash_sha256_update,
 		git_hash_sha256_final,
 		&empty_tree_oid_sha256,
diff --git a/sha256/gcrypt.h b/sha256/gcrypt.h
index 09bd8bb200..501da5ed91 100644
--- a/sha256/gcrypt.h
+++ b/sha256/gcrypt.h
@@ -22,8 +22,14 @@ inline void gcrypt_SHA256_Final(unsigned char *digest, gcrypt_SHA256_CTX *ctx)
 	memcpy(digest, gcry_md_read(*ctx, GCRY_MD_SHA256), SHA256_DIGEST_SIZE);
 }
 
+inline void gcrypt_SHA256_Clone(gcrypt_SHA256_CTX *dst, const gcrypt_SHA256_CTX *src)
+{
+	gcry_md_copy(dst, *src);
+}
+
 #define platform_SHA256_CTX gcrypt_SHA256_CTX
 #define platform_SHA256_Init gcrypt_SHA256_Init
+#define platform_SHA256_Clone gcrypt_SHA256_Clone
 #define platform_SHA256_Update gcrypt_SHA256_Update
 #define platform_SHA256_Final gcrypt_SHA256_Final