From: Junio C Hamano Date: Tue, 04 Nov 2025 10:24:00 GMT Subject: Re: [PATCH 06/14] hash: add a function to look up hash algo structs Message-ID: In-Reply-To: "brian m. carlson" writes: >> > +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, ... >> 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.