Re: [PATCH 1/4] refs: add referent parameter to refs_resolve_ref_unsafe

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 10, 2024, 18:09 UTC
Message-ID: <xmqqy17c3c1v.fsf@gitster.g>
In-Reply-To: <ZmarVcF5JjsZx0dl@tanuki>

Patrick Steinhardt <ps@pks.im> writes:

> I wonder whether we can future-proof the code a bit more by introducing
> a struct that contains everything we want to pass to the callback
> function.

That would hopefully make a change with a large blast a one-time event. But at the same time, it may end up making it too opaque and hard to verify if all the API functions are using/updating/verifying all the members of the struct as they should. Compared to that, unused parameters are easier to verify mechanically by compilers.

> This would also allow us to get rid of this awful `peel_iterated_oid()`
> function that relies on global state in many places, as we can put the
> peeled OID into that structure, as well.

Yes, such a benefit may justify a one-time "affect many callers" event. Or the underlying for_each_*() friend of functions can be updated to use a single struct and then the current "only selected parameters that are used ar passed" API can be made into a set of thin wrappers around it, and then callers can be converted one step at a time, in a multi-step series, perhaps.