Re: [PATCH 06/14] hash: add a function to look up hash algo structs

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 4, 2025, 10:24 UTC
Message-ID: <xmqqv7jqqenz.fsf@gitster.g>
In-Reply-To: <aQlbZRAKq9RwYnqe@fruit.crustytoothpaste.net>

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

Show 18 quoted lines

>> > +const struct git_hash_algo *hash_algo_ptr_by_offset(uint32_t algo)
>> > +{
>> > +	return &hash_algos[algo];
>> > +}
>> 
>> Hmph, technically "algo" may be an "offset" into the array, but I'd
>> consider it an implementation detail.  We have hash_algo instances
>> floating somewhere in-core, and have a way to obtain a pointer to
>> one of these instances by "algorithm number".  For the user of the
>> API, the fact that these instances are stored in contiguous pieces
>> of memory as an array of struct is totally irrelevant.  For that
>> reason, I was somewhat repelled by the "by-offset" part of the
>> function name.
>
> I fear I don't have a better name.  "by_id" is the format ID.  I could
> write "hash_algo_ptr_by_hash_algo" but that seems slightly bizarre and
> difficult to type.  I could do "by_index", but you might have the same
> objection to that name.  Would you like to propose a nicer alternative?

const struct git_hash_algo *hash_algo_ptr_by_algo_number(uint32_t algo_num)
{
	return &hash_algos[algo_num];
}

Then, ...

Show 9 quoted lines

>> The next function ...
>> 
>> >  uint32_t hash_algo_by_name(const char *name)
>> 
>> ... calls what it returns "hash_algo", but the "hash_algo" returned
>> by this new function is quite different.  One is just the "algorithm
>> number", while the other is "algorithm instance".  Perhaps calling
>> both with the same name "hash algo" is the true source of confusing
>> naming of this new function?

... would become

uint32_t hash_algo_num_by_name(const char *name)

perhaps.