Subject: Re: [PATCH 02/17] midx: split `get_midx_checksum()` by adding `get_midx_hash()`

From: Patrick Steinhardt <[email-masked]>
Date: Mon, 8 Dec 2025 19:25:05 +0100

On Sat, Dec 06, 2025 at 03:31:04PM -0500, Taylor Blau wrote:
> When trying to print out, say, the hexadecimal representation of a
> MIDX's hash, our code will do something like:
> 
>     hash_to_hex_algop(get_midx_checksum(m),
>                       m->source->odb->repo->hash_algo);
> 
> , which is both cumbersome and repetitive. In fact, all but a handful of
> callers to `get_midx_checksum()` do exactly the above. Reduce the
> repetitive nature of calling `get_midx_checksum()` by having it return a
> pointer into a static buffer containing the above result.
> 
> For the handful of callers that do need to compare the raw bytes and
> don't want to deal with an encoded copy (e.g., because they are passing
> it to hasheq() or similar), introduce `get_midx_hash()` which returns
> the raw bytes.

This is a welcome change indeed.

> diff --git a/midx.h b/midx.h
> index 7c7e0b59121..e188ffeb578 100644
> --- a/midx.h
> +++ b/midx.h
> @@ -85,7 +85,8 @@ struct multi_pack_index {
>  #define MIDX_EXT_BITMAP "bitmap"
>  #define MIDX_EXT_MIDX "midx"
>  
> -const unsigned char *get_midx_checksum(const struct multi_pack_index *m);
> +const char *get_midx_checksum(const struct multi_pack_index *m) /* static buffer */;
> +const unsigned char *get_midx_hash(const struct multi_pack_index *m);
>  void get_midx_filename(struct odb_source *source, struct strbuf *out);
>  void get_midx_filename_ext(struct odb_source *source, struct strbuf *out,
>  			   const unsigned char *hash, const char *ext);

If I didn't have the context of this patch series I would be wondering
what the actual difference between `get_midx_checksum()` and
`get_midx_hash()` is. The way the functions are named seems to rather
indicate that we talk about two different kinds of hashes, rather than
two different ways to encode them.

Would it maybe be preferable to call them `get_midx_checksum()` and
`get_midx_checksum_hex()`? While at it, we could go even further and
rename them to `midx_get_checksum()` and `midx_get_checksum_hex()` to
conform to our modern best practices.

Patrick