Re: [RFC PATCH 3/6] hex: make hex_to_bytes accept kind of hex to use

[Jeff King <peff@peff.net>]

On Fri, Jul 31, 2026 at 12:38:17AM -0700, Junio C Hamano wrote:

> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
> 
> > -int hex_to_bytes(unsigned char *binary, const char *hex, size_t len)
> > +int hex_to_bytes(unsigned char *binary, const char *hex, size_t len, enum hexkind kind)
> >  {
> >  	for (; len; len--, hex += 2) {
> > -		unsigned int val = (hexval(hex[0], HEX_KIND_MIXED) << 4) | hexval(hex[1], HEX_KIND_MIXED);
> > +		unsigned int val = (hexval(hex[0], kind) << 4) | hexval(hex[1], kind);
> >  
> >  		if (val & ~0xff)
> >  			return -1;
> 
> It depends on how big 'len' would be to matter, but if we are
> looping for a long stretch, choosing which one of the two hexval
> tables to use outside the loop and using that inside may of course
> be more performant.
> 
> I wondered how ugly such a restructure of the API would look like,
> and it does not look _too_ bad.
> 
> 	void *hextable = hex_table(HEX_KIND_MIXED);
> 
> 	for (; len; len--, hex += 2) {
> 		unsigned int val =
> 			(hexval(hex[0], hextable) << 4) | hexval(hex[1], hextable);
> 		...
> 	}
> 
> The true type of hextable would be "signed char [256]", but the
> callers of the hexval() function do not need to know it, hence I
> chose "void *" here.

I had the same thought when reading this, but I wondered if the compiler
might be able to hoist the comparison out of the loop itself (because
hexval() it inlined anyway). It doesn't seem to do so, though (at least
with gcc-15). It loads both table addresses into registers, but there's
still a branch in the loop to decide which table to use.

So in theory this kind of manual hoisting could help.  Might not be that
big a deal with branch prediction, though.

-Peff