[PATCH 9/9] hash: add platform-specific discard functions

[Jeff King <peff@peff.net>]

Our git_hash_discard() is a bit hacky: it just calls git_hash_final()
into a dummy result buffer, using the side effect that each
implementation's Final() function will also free any resources.

This is probably not too terrible, since generating the final hash is
not that expensive and we'd mostly call discard on unusual or error code
paths. But we can do better by widening the platform API a bit to add an
explicit discard function.

This requires an annoying amount of boilerplate:

  - Each algorithm needs a git_$ALGO_discard() wrapper that dereferences
    the union'd git_hash_ctx into the type-safe field. So sha1 + sha256
    + sha1-unsafe, plus a BUG() for the unknown algo. And then these all
    need to be referenced in the git_hash_algo structs.

  - Platforms which don't do anything special to discard now need a
    fallback function which does nothing. And we need this for each algo
    (sha1, sha256, and sha1-unsafe).

  - Platforms which do need to discard must define their discard
    functions. This includes sha1/openssl, sha256/openssl, and
    sha256/gcrypt (no sha1-unsafe here as it sits atop the sha1/openssl
    functions).

  - Algo selection needs to point platform_*_Discard to the appropriate
    underlying macro, or indicate that the fallback should be used. We
    have a similar situation for the Clone function (where a straight
    memcpy() of the context struct is not enough for some platforms).
    I've tied Discard to the same flag used by Clone here, since they
    are basically the same problem: is the hash context a sequence of
    bytes, or does it need smart copying/discarding?

It's easy to miss a case here since we don't even compile the
implementations we aren't using. I've tested with each of:

  - no flags, which uses our internal sha1/sha256 implementations, both
    of which exercise the noop fallback function

  - OPENSSL_SHA1_UNSAFE=1, which checks that our unsafe macro
    redirections work

  - OPENSSL_SHA1=1, though you should not do that in real life!

  - OPENSSL_SHA256=1, passes tests with GIT_TEST_DEFAULT_HASH=sha256

  - GCRYPT_SHA256=1, which likewise passes

The other implementations do not set the CLONE_HELPER flag, so they
treat the context as bytes and should be fine with the fallback.

Signed-off-by: Jeff King <peff@peff.net>
---
One of the reasons I left this to the end is that I wasn't sure it would
be worth it. I think it probably is (hence posting it), but we could
live with the hacky implementation forever if we wanted. :)

It also prompted me to test with all of the backends I could build,
which is how I found the unrelated gcrypt leak fixed in patch 8.

 hash.c           | 33 +++++++++++++++++++++++++--------
 hash.h           | 21 +++++++++++++++++++++
 sha1/openssl.h   |  6 ++++++
 sha256/gcrypt.h  |  6 ++++++
 sha256/openssl.h |  6 ++++++
 5 files changed, 64 insertions(+), 8 deletions(-)

diff --git a/hash.c b/hash.c
index 63672a3d22..55d1d41770 100644
--- a/hash.c
+++ b/hash.c
@@ -72,6 +72,11 @@ static void git_hash_sha1_final_oid(struct object_id *oid, struct git_hash_ctx *
 	oid->algo = GIT_HASH_SHA1;
 }
 
+static void git_hash_sha1_discard(struct git_hash_ctx *ctx)
+{
+	git_SHA1_Discard(&ctx->state.sha1);
+}
+
 static void git_hash_sha1_init_unsafe(struct git_hash_ctx *ctx)
 {
 	ctx->algop = unsafe_hash_algo(&hash_algos[GIT_HASH_SHA1]);
@@ -102,6 +107,11 @@ static void git_hash_sha1_final_oid_unsafe(struct object_id *oid, struct git_has
 	oid->algo = GIT_HASH_SHA1;
 }
 
+static void git_hash_sha1_discard_unsafe(struct git_hash_ctx *ctx)
+{
+	git_SHA1_Discard_unsafe(&ctx->state.sha1_unsafe);
+}
+
 static void git_hash_sha256_init(struct git_hash_ctx *ctx)
 {
 	ctx->algop = unsafe_hash_algo(&hash_algos[GIT_HASH_SHA256]);
@@ -135,6 +145,11 @@ static void git_hash_sha256_final_oid(struct object_id *oid, struct git_hash_ctx
 	oid->algo = GIT_HASH_SHA256;
 }
 
+static void git_hash_sha256_discard(struct git_hash_ctx *ctx)
+{
+	git_SHA256_Discard(&ctx->state.sha256);
+}
+
 static void git_hash_unknown_init(struct git_hash_ctx *ctx UNUSED)
 {
 	BUG("trying to init unknown hash");
@@ -165,6 +180,11 @@ static void git_hash_unknown_final_oid(struct object_id *oid UNUSED,
 	BUG("trying to finalize unknown hash");
 }
 
+static void git_hash_unknown_discard(struct git_hash_ctx *ctx UNUSED)
+{
+	BUG("trying to discard unknown hash");
+}
+
 static const struct git_hash_algo sha1_unsafe_algo = {
 	.name = "sha1",
 	.format_id = GIT_SHA1_FORMAT_ID,
@@ -176,6 +196,7 @@ static const struct git_hash_algo sha1_unsafe_algo = {
 	.update_fn = git_hash_sha1_update_unsafe,
 	.final_fn = git_hash_sha1_final_unsafe,
 	.final_oid_fn = git_hash_sha1_final_oid_unsafe,
+	.discard_fn = git_hash_sha1_discard_unsafe,
 	.empty_tree = &empty_tree_oid,
 	.empty_blob = &empty_blob_oid,
 	.null_oid = &null_oid_sha1,
@@ -193,6 +214,7 @@ const struct git_hash_algo hash_algos[GIT_HASH_NALGOS] = {
 		.update_fn = git_hash_unknown_update,
 		.final_fn = git_hash_unknown_final,
 		.final_oid_fn = git_hash_unknown_final_oid,
+		.discard_fn = git_hash_unknown_discard,
 		.empty_tree = NULL,
 		.empty_blob = NULL,
 		.null_oid = NULL,
@@ -208,6 +230,7 @@ const struct git_hash_algo hash_algos[GIT_HASH_NALGOS] = {
 		.update_fn = git_hash_sha1_update,
 		.final_fn = git_hash_sha1_final,
 		.final_oid_fn = git_hash_sha1_final_oid,
+		.discard_fn = git_hash_sha1_discard,
 		.unsafe = &sha1_unsafe_algo,
 		.empty_tree = &empty_tree_oid,
 		.empty_blob = &empty_blob_oid,
@@ -224,6 +247,7 @@ const struct git_hash_algo hash_algos[GIT_HASH_NALGOS] = {
 		.update_fn = git_hash_sha256_update,
 		.final_fn = git_hash_sha256_final,
 		.final_oid_fn = git_hash_sha256_final_oid,
+		.discard_fn = git_hash_sha256_discard,
 		.empty_tree = &empty_tree_oid_sha256,
 		.empty_blob = &empty_blob_oid_sha256,
 		.null_oid = &null_oid_sha256,
@@ -285,14 +309,7 @@ void git_hash_final_oid(struct object_id *oid, struct git_hash_ctx *ctx)
 
 void git_hash_discard(struct git_hash_ctx *ctx)
 {
-	/*
-	 * XXX Many implementations do not need to do anything here,
-	 * and a dummy final() call is wasteful. But we can't fix
-	 * that unless our implementation API exposes a discard
-	 * primitive.
-	 */
-	unsigned char dummy[GIT_MAX_RAWSZ];
-	git_hash_final(dummy, ctx);
+	ctx->algop->discard_fn(ctx);
 }
 
 uint32_t hash_algo_by_name(const char *name)
diff --git a/hash.h b/hash.h
index 6b2f04e2a4..0a23ef4dfd 100644
--- a/hash.h
+++ b/hash.h
@@ -37,6 +37,7 @@
 #    define platform_SHA1_Clone_unsafe openssl_SHA1_Clone
 #    define platform_SHA1_Update_unsafe openssl_SHA1_Update
 #    define platform_SHA1_Final_unsafe openssl_SHA1_Final
+#    define platform_SHA1_Discard_unsafe openssl_SHA1_Discard
 #  else
 #    define platform_SHA_CTX_unsafe SHA_CTX
 #    define platform_SHA1_Init_unsafe SHA1_Init
@@ -92,6 +93,7 @@
 #  define platform_SHA1_Final_unsafe   platform_SHA1_Final
 #  ifdef platform_SHA1_Clone
 #    define platform_SHA1_Clone_unsafe platform_SHA1_Clone
+#    define platform_SHA1_Discard_unsafe platform_SHA1_Discard
 #  endif
 #  ifdef SHA1_NEEDS_CLONE_HELPER
 #    define SHA1_NEEDS_CLONE_HELPER_UNSAFE
@@ -110,9 +112,11 @@
 
 #ifdef platform_SHA1_Clone
 #define git_SHA1_Clone	platform_SHA1_Clone
+#define git_SHA1_Discard platform_SHA1_Discard
 #endif
 #ifdef platform_SHA1_Clone_unsafe
 #  define git_SHA1_Clone_unsafe platform_SHA1_Clone_unsafe
+#  define git_SHA1_Discard_unsafe platform_SHA1_Discard_unsafe
 #endif
 
 #ifndef platform_SHA256_CTX
@@ -129,6 +133,7 @@
 
 #ifdef platform_SHA256_Clone
 #define git_SHA256_Clone	platform_SHA256_Clone
+#define git_SHA256_Discard	platform_SHA256_Discard
 #endif
 
 #ifdef SHA1_MAX_BLOCK_SIZE
@@ -142,20 +147,32 @@ static inline void git_SHA1_Clone(git_SHA_CTX *dst, const git_SHA_CTX *src)
 {
 	memcpy(dst, src, sizeof(*dst));
 }
+static inline void git_SHA1_Discard(git_SHA_CTX *ctx UNUSED)
+{
+	/* noop */
+}
 #endif
 #ifndef SHA1_NEEDS_CLONE_HELPER_UNSAFE
 static inline void git_SHA1_Clone_unsafe(git_SHA_CTX_unsafe *dst,
 				       const git_SHA_CTX_unsafe *src)
 {
 	memcpy(dst, src, sizeof(*dst));
 }
+static inline void git_SHA1_Discard_unsafe(git_SHA_CTX_unsafe *ctx UNUSED)
+{
+	/* noop */
+}
 #endif
 
 #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));
 }
+static inline void git_SHA256_Discard(git_SHA256_CTX *ctx UNUSED)
+{
+	/* noop */
+}
 #endif
 
 /*
@@ -271,6 +288,7 @@ typedef void (*git_hash_clone_fn)(struct git_hash_ctx *dst, const struct git_has
 typedef void (*git_hash_update_fn)(struct git_hash_ctx *ctx, const void *in, size_t len);
 typedef void (*git_hash_final_fn)(unsigned char *hash, struct git_hash_ctx *ctx);
 typedef void (*git_hash_final_oid_fn)(struct object_id *oid, struct git_hash_ctx *ctx);
+typedef void (*git_hash_discard_fn)(struct git_hash_ctx *ctx);
 
 struct git_hash_algo {
 	/*
@@ -306,6 +324,9 @@ struct git_hash_algo {
 	/* The hash finalization function for object IDs. */
 	git_hash_final_oid_fn final_oid_fn;
 
+	/* Discard an initialized hash without finalizing. */
+	git_hash_discard_fn discard_fn;
+
 	/* The OID of the empty tree. */
 	const struct object_id *empty_tree;
 
diff --git a/sha1/openssl.h b/sha1/openssl.h
index 1038af47da..48deeb724a 100644
--- a/sha1/openssl.h
+++ b/sha1/openssl.h
@@ -40,12 +40,18 @@ static inline void openssl_SHA1_Clone(struct openssl_SHA1_CTX *dst,
 	EVP_MD_CTX_copy_ex(dst->ectx, src->ectx);
 }
 
+static inline void openssl_SHA1_Discard(struct openssl_SHA1_CTX *ctx)
+{
+	EVP_MD_CTX_free(ctx->ectx);
+}
+
 #ifndef platform_SHA_CTX
 #define platform_SHA_CTX openssl_SHA1_CTX
 #define platform_SHA1_Init openssl_SHA1_Init
 #define platform_SHA1_Clone openssl_SHA1_Clone
 #define platform_SHA1_Update openssl_SHA1_Update
 #define platform_SHA1_Final openssl_SHA1_Final
+#define platform_SHA1_Discard openssl_SHA1_Discard
 #endif
 
 #endif /* SHA1_OPENSSL_H */
diff --git a/sha256/gcrypt.h b/sha256/gcrypt.h
index 694a2b70a1..d91ffe73d3 100644
--- a/sha256/gcrypt.h
+++ b/sha256/gcrypt.h
@@ -31,10 +31,16 @@ static inline void gcrypt_SHA256_Clone(gcrypt_SHA256_CTX *dst, const gcrypt_SHA2
 	gcry_md_copy(dst, *src);
 }
 
+static inline void gcrypt_SHA256_Discard(gcrypt_SHA256_CTX *ctx)
+{
+	gcry_md_close(*ctx);
+}
+
 #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
+#define platform_SHA256_Discard gcrypt_SHA256_Discard
 
 #endif
diff --git a/sha256/openssl.h b/sha256/openssl.h
index c1083d9491..3d457ca99d 100644
--- a/sha256/openssl.h
+++ b/sha256/openssl.h
@@ -40,10 +40,16 @@ static inline void openssl_SHA256_Clone(struct openssl_SHA256_CTX *dst,
 	EVP_MD_CTX_copy_ex(dst->ectx, src->ectx);
 }
 
+static inline void openssl_SHA256_Discard(struct openssl_SHA256_CTX *ctx)
+{
+	EVP_MD_CTX_free(ctx->ectx);
+}
+
 #define platform_SHA256_CTX openssl_SHA256_CTX
 #define platform_SHA256_Init openssl_SHA256_Init
 #define platform_SHA256_Clone openssl_SHA256_Clone
 #define platform_SHA256_Update openssl_SHA256_Update
 #define platform_SHA256_Final openssl_SHA256_Final
+#define platform_SHA256_Discard openssl_SHA256_Discard
 
 #endif /* SHA256_OPENSSL_H */
-- 
2.55.0.418.g37da59dd42