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é
