On Wed, Jul 29, 2026 at 4:33 PM brian m. carlson
<sandals@xxxxxxxxxxxxxxxxxxxx> 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.
[...]
> - 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?