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

["René Scharfe" <l.s.r@web.de>]

Am 01.11.2017 um 20:55 schrieb Jeff King:
> 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'm confused by the word "many".  After this series there are three
callers of hex_to_bytes() and I don't expect that number to grow.

Would loose objects be stored at paths containing only a subset of their
new hash value?  If they won't then there will be two acceptable lengths
instead of the one we have today, which should be easy to handle.

> 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".

Hmm, ignoring any slashes is an interesting idea.  Is that too loose, I
wonder?  And I don't think it makes for a good primitive, as slashes
only need to be ignored in a single place so far (here in http-push.c).
Collecting special cases in a central place, guarded by flags, doesn't
reduce the overall complexity.

> 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.

How so?  If you have a buffer then you need to know the size of the
data to copy into it as well, or you'll learn it in the process.

The call sites of hex_to_bytes() have to be modified along with the
functions in hex.c to support longer hashes, with or without this
series.

René