Re: [PATCH 03/14] hash: use uint32_t for object_id algorithm

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 28, 2025, 19:33 UTC
Message-ID: <xmqqh5viddo3.fsf@gitster.g>
In-Reply-To: <aQCKCfuaEKBArD-g@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

Show 9 quoted lines

> On Mon, Oct 27, 2025 at 12:43:53AM +0000, brian m. carlson wrote:
>> We currently use an int for this value, but we'll define this structure
>> from Rust in a future commit and we want to ensure that our data types
>> are exactly identical.  To make that possible, use a uint32_t for the
>> hash algorithm.
>
> An alternative would be to introduce an enum and set up bindgen so that
> we can pull this enum into Rust. I'd personally favor that over using an
> uint32_t as it conveys way more meaning. Have you considered this?

Yeah, I do not very much appreciate change from "int" to "uint32_t" randomly done only for things that happen to be used by both C and Rust. "When should I use 'int' or 'unsigned' and when should I use 'uint32_t'?" becomes extremely hard to answer.

I suspect that it would be much more palatable if these functions and struct members are to use a distinct type that is used only by hash algorithm number (your "enum" is fine), that is typedef'ed to be the 32-bit unsigned integer, e.g,

    +typedef uint32_t hash_algo_type;
    -int hash_algo_by_name(const char *name)
    +hash_algo_type hash_algo_by_name(const char *name)

Yeah, I know that C does not give us type safety against mixing two different things, both of which are typedef'ed to the same uint32_t, but doing something like the above would still add documentation value.