On 2026-08-25 at 19:44:34, Elijah Newren wrote:
> On Wed, Jul 29, 2026 at 4:33 PM brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
> >
> > We've open-coded a different implementation of parsing hex values here
> > when we already have a perfectly good one in hexval.  This
> > implementation will almost certainly be slower because it isn't
> > table-driven, unlike the other one, and since it's not constant time it
> > has no other advantages either.  To tidy things up and prepare for
> > future work, switch to hexval in this case.
> 
> As Junio noted, you may want to call out that your replacement drops
> the case-normalization that the former parse_oid_prefix() provided.

Will fix in v2.

> [...]
> > -               unsigned char val;
> [...]
> > +               int val = hexval(c, HEX_KIND_OID);
> > +
> > +               if (val < 0)
> >                         return -1;
> [...]
> >                if (oid_out) {
> >                        if (!(i & 1))
> >                                val <<= 4;
> >                        oid_out->hash[i >> 1] |= val;
> 
> hexval returns unsigned int.  Is there a risk that someone "tries to
> fix" that discrepancy by changing val to unsigned int here,
> inadvertently causing the `if` immediately below to become dead code?
> 
> In patch 1, in hex2chr, you used a (val & ~0xf) check together with an
> unsigned int val; would that make sense here, or is that overkill?

I can re-roll with an appropriate change, sure.  I think that we'd need
to have a slightly different check, but I'll tidy it up accordingly.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA