Re: [PATCH v2 4/8] hash.h: introduce `unsafe_hash_algo()`

[Patrick Steinhardt <ps@pks.im>]

On Wed, Jan 08, 2025 at 02:14:42PM -0500, Taylor Blau wrote:
> Address these issues by introducing a new pattern whereby one
> 'git_hash_algo' can return a pointer to another 'git_hash_algo' that
> represents the unsafe version of itself. So instead of having something
> like:
> 
>     if (unsafe)
>       the_hash_algo->init_fn(...);
>       the_hash_algo->update_fn(...);
>       the_hash_algo->final_fn(...);
>     else
>       the_hash_algo->unsafe_init_fn(...);
>       the_hash_algo->unsafe_update_fn(...);
>       the_hash_algo->unsafe_final_fn(...);
> 
> we can instead write:
> 
>     struct git_hash_algo *algop = the_hash_algo;
>     if (unsafe)
>       algop = unsafe_hash_algo(algop);
> 
>     the_hash_algo->init_fn(...);
>     the_hash_algo->update_fn(...);
>     the_hash_algo->final_fn(...);

This should all be `algop->init_fn(...)` and so on, right?

> This removes the existing shortcoming by no longer forcing the caller to
> "remember" which variant of the hash functions it wants to call, only to
> hold onto a 'struct git_hash_algo' pointer that is initialized once.

We still have to remember something, but now we only have to remember
the pointer to the hash algorithm itself, which definitely is less
tedious.

For what it's worth, this prompted me to have another look at my
proposal to stop having to use the hash algo altogether for `update()`
et al, and now have a version that works. It builds on top of your patch
series, which still is a step into the right direction. So I'll send it
once your series is being merged down.

Thanks!

Patrick