On 28/01/2018 16:57, brian m. carlson wrote:
In various parts of our code, we want to allocate a structure
representing the internal state of a hash algorithm. The original
implementation of the hash algorithm abstraction assumed we would do
that using heap allocations, and added a context size element to struct
git_hash_algo. However, most of the existing code uses stack
allocations and conversion would needlessly complicate various parts of
the code. Add a union for the purpose of allocating hash contexts on
the stack and a typedef for ease of use. Remove the ctxsz element for
struct git_hash_algo, which is no longer very useful.
Overall, I am OK with this approach (it's straightforward change and
cleanest way to replace direct calls to git_SHA1_* functions), but just
to play devil's advocate: OpenSSL decided to sway users into heap
allocated contexts, citing binary compatibility issues if they change
the size of context structure. [1]
I think we might need to revisit this design decision in future -
perhaps as soon as we'll transition away from calling git_SHA1_*
functions directly.
+/* A suitably aligned type for stack allocations of hash contexts. */
+union git_hash_ctx {
+ git_SHA_CTX sha1;
+};
+typedef union git_hash_ctx git_hash_ctx;
+
typedef void (*git_hash_init_fn)(void *ctx);
typedef void (*git_hash_update_fn)(void *ctx, const void *in, size_t len);
typedef void (*git_hash_final_fn)(unsigned char *hash, void *ctx);
I think it would be appropriate to replace "void *ctx" with
"git_hash_ctx *ctx". This way we can avoid unnecessary casting in
git_hash_sha1_* functions.
[1] https://wiki.openssl.org/index.php/Manual:EVP_DigestInit(3)#NOTES
--
| ← Ceci n'est pas une pipe
Patryk Obara
