From: Patrick Steinhardt <ps@pks.im>
Date: Tue, 28 Oct 2025 09:18:12 GMT
Subject: Re: [PATCH 06/14] hash: add a function to look up hash algo structs
Message-ID: <aQCKVGmhekVBthTh@pks.im>
In-Reply-To: <20251027004404.2152927-7-sandals@crustytoothpaste.net>

On Mon, Oct 27, 2025 at 12:43:56AM +0000, brian m. carlson wrote:
> diff --git a/hash.c b/hash.c
> index 81b4f87027..2f4e88e501 100644
> --- a/hash.c
> +++ b/hash.c
> @@ -241,6 +241,11 @@ const char *empty_tree_oid_hex(const struct git_hash_algo *algop)
>  	return oid_to_hex_r(buf, algop->empty_tree);
>  }
>  
> +const struct git_hash_algo *hash_algo_ptr_by_offset(uint32_t algo)
> +{
> +	return &hash_algos[algo];
> +}

I think we should have some safety mechanisms here to verify that we
don't cause an out-of-bounds access.

> diff --git a/src/hash.rs b/src/hash.rs
> index 1b9f07489e..a5b9493bd8 100644
> --- a/src/hash.rs
> +++ b/src/hash.rs
> @@ -160,4 +162,17 @@ impl HashAlgorithm {
>              HashAlgorithm::SHA256 => &Self::SHA256_NULL_OID,
>          }
>      }
> +
> +    /// A pointer to the C `struct git_hash_algo` for interoperability with C.
> +    pub fn hash_algo_ptr(self) -> *const c_void {
> +        unsafe { c::hash_algo_ptr_by_offset(self as u32) }
> +    }
> +}
> +
> +pub mod c {
> +    use std::os::raw::c_void;
> +
> +    extern "C" {
> +        pub fn hash_algo_ptr_by_offset(n: u32) -> *const c_void;
> +    }
>  }

I guess eventually we should replace such declarations via bindgen. If
so, we could also pull in the `struct git_hash_algo` declaration and
have the function reutrn that structure instead of a oid pointer.

Patrick