Subject: Re: [PATCH 01/14] oidtree: modernize the code a bit

From: "brian m. carlson" <[email-masked]>
Date: Fri, 20 Mar 2026 22:30:36 +0000

On 2026-03-20 at 06:40:10, Patrick Steinhardt wrote:
> On Thu, Mar 19, 2026 at 09:08:44AM -0700, Junio C Hamano wrote:
> > I know the original also used GIT_MAX_HEXSZ to clamp the length for
> > sanity, but because we know what algorithm is in use, I wonder if we
> > want to use the limit more specific to it.
> 
> That assumes that the passed prefix OID actually has an algorithm
> attached to it, and that may not be the case. We could initialize the
> overall oidtree with a hash algorithm in `oidtree_init()`, and if so we
> can then become a bit more thorough with our asserts.
>
> But I feel like that would go beyond the smallish cleanups that I'm
> doing in this patch.

We should stop assuming that a zero `algo` field in `struct object_id`
means `the_hash_algo` because that makes libification hard and our Rust
code doesn't support it (because accessing mutable globals without a
lock is unsafe)[0].  So in general, I would be fine with forcing callers
to set an algorithm per OID, both here and elsewhere in our code.

However, I am also fine with doing that in a different series for the
sake of minimalism in this one.  I will probably get to that at some
point if nobody else does.

[0] Rust also typically initializes all fields explicitly (and
zero-initialization is also unsafe), so there's no urge to be lazy and
do `memset(p, 0, sizeof(*p))`, which is the usual source of the zero
`algo` fields in our codebase.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA