Re: [RFC PATCH 5/6] object-name: use hexval

From: Elijah Newren <newren@gmail.com>
Date: Aug 25, 2026, 19:44 UTC
Message-ID: <CABPp-BFDaWdahoOnNRGQjshzQXin1YLuROv94W_PrajnLWDAuQ@mail.gmail.com>
In-Reply-To: <20260729233215.398654-6-sandals@crustytoothpaste.net>

On Wed, Jul 29, 2026 at 4:33 PM brian m. carlson <sandals@crustytoothpaste.net> wrote:

Show 7 quoted lines

>
> 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.

[...]

> -               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?