Re: [RFC PATCH 1/6] hex: add functionality for lowercase-only hex

["brian m. carlson" <sandals@crustytoothpaste.net>]

[-- Attachment #1: Type: text/plain, Size: 2181 bytes --]

On 2026-08-25 at 15:39:43, Junio C Hamano wrote:
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
> 
> > We currently allow both upper and lower case for all hex values in Git.
> > However, in a future commit, we'll want to change that to allow only
> > lowercase values in some cases.  To prepare for that case, provide a
> > table to convert hex values using lowercase only and an enum to let us
> > choose which we want, wiring it up to the hexval function.
> >
> > For now, keep things completely the same by specifying only the
> > variant that accepts both lowercase and uppercase to avoid changing
> > behavior.
> >
> > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> > ---
> >  color.c    |  2 +-
> >  hex-ll.c   | 37 ++++++++++++++++++++++++++++++++++++-
> >  hex-ll.h   | 14 ++++++++++----
> >  pkt-line.c |  8 ++++----
> >  4 files changed, 51 insertions(+), 10 deletions(-)
> 
> 
> Now this is an embarrassingly late review.  I hope this is not a
> sign that nobody is paying attention on the list these days X-<.
> 
> > +const signed char hexval_lc_table[256] = {
> > +	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 00-07 */
> > +	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 08-0f */
> > +	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 10-17 */
> > +	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 18-1f */
> > +	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 20-27 */
> > +	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 28-2f */
> > +	  0,  1,  2,  3,  4,  5,  6,  7,		/* 30-37 */
> > +	  8,  9, -1, -1, -1, -1, -1, -1,		/* 38-3f */
> > +	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 40-47 */
> > +	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 48-4f */
> > +	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 50-57 */
> > +	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 58-5f */
> > +	 -1, 10, 11, 12, 13, 14, 15, -1,		/* 60-67 */
> > +	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 68-67 */
> 
> That's 68-6f if I am not mistaken ;-).

So it is.  Will fix in v2.

I think I accidentally included the uppercase but not lowercase variants
when creating the original array and then copied and pasted the line,
but messed up the comment.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 325 bytes --]