On Wed, Nov 01, 2017 at 10:59:49PM +0100, René Scharfe wrote: > > 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.
I meant only that most callers that parse oids, both in-file and not, would want to stop knowing about the length ahead of time. I think parse_oid_hex() solves that problem for most callers. > 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 don't know. TBH, I'm not sure anyone has much interest in making http-push work with new hashes. I'd be OK if it simply doesn't until somebody interested shows up to change that. > > 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. You have to know how big the data you have is, but you don't necessarily know whether that makes a complete hash or not. With a "remove slashes and then parse" strategy, you can do the removing without worrying about how big things are _supposed_ to be, and then the parser can tell you if you have a valid oid or not. The logic for what a hash looks like _and_ how big it must be are both in the parser. With the new code you have here, we have to be a bit more intimate with SHA1_HEXSZ in the calling code. It knows that the hash consists of a certain number of hex bytes. I'm perfectly willing to punt on it for now. I'm not sure we know 100% yet what "new"-style hashes will look like, nor how their loose-object filenames would look. -Peff
