Re: [PATCH 2/3] http-push: use hex_to_bytes()

[Jeff King <peff@peff.net>]

On Tue, Oct 31, 2017 at 02:49:56PM +0100, René Scharfe wrote:

> The path of a loose object contains its hash value encoded into two
> substrings of hexadecimal digits, separated by a slash.  The current
> code copies the pieces into a temporary buffer to get rid of the slash
> and then uses get_oid_hex() to decode the hash value.
> 
> Avoid the copy by using hex_to_bytes() directly on the substrings.
> That's shorter and easier.
> 
> While at it correct the length of the second substring in a comment.

I think the patch is correct, but I wonder if this approach will bite us
later on when we start dealing with multiple lengths of hashes.

The hex_to_bytes() function requires that the caller make sure they have
the right number of bytes. But for many callers, I think they'd want to
say "parse this oid, which might be truncated; I can't tell what the
length is supposed to be".

I.e., I wonder if the right primitive is really something like
parse_oid_hex(), but with a flag to say "skip over interior slashes".

We don't need to deal with that eventuality yet, but I'm on the fence on
whether this patch is making that harder down the road or not. The
current strategy of "stuff it into a buffer without slashes" would be
easier to convert, I think.

-Peff