John Naylor <johncnaylorls@gmail.com> writes:
> Okay, I added a comment. I also agree with Michael that my quick
> one-off was a bit hard to read so I've cleaned it up a bit. I plan to
> commit the attached by Friday, along with any bikeshedding that
> happens by then.
Couple of thoughts:
1. I was actually hoping for a comment on the constant's definition,
perhaps along the lines of
/*
* The hex expansion of each possible byte value (two chars per value).
*/
2. Since "src" is defined as "const char *", I'm pretty sure that
pickier compilers will complain that
+ unsigned char usrc = *((unsigned char *) src);
results in casting away const. Recommend
+ unsigned char usrc = *((const unsigned char *) src);
3. I really wonder if
+ memcpy(dst, &hextbl[2 * usrc], 2);
is faster than copying the two bytes manually, along the lines of
+ *dst++ = hextbl[2 * usrc];
+ *dst++ = hextbl[2 * usrc + 1];
Compilers that inline memcpy() may arrive at the same machine code,
but why rely on the compiler to make that optimization? If the
compiler fails to do so, an out-of-line memcpy() call will surely
be a loser.
A variant could be
+ const char *hexptr = &hextbl[2 * usrc];
+ *dst++ = hexptr[0];
+ *dst++ = hexptr[1];
but this supposes that the compiler fails to see the common
subexpression in the other formulation, which I believe
most modern compilers will see.
regards, tom lane
