On Tue, Oct 04, 2016 at 02:46:44PM -0700, Junio C Hamano wrote: > Jacob Keller <jacob.keller@xxxxxxxxx> writes: > > > On Mon, Oct 3, 2016 at 1:35 PM, Jeff King <peff@xxxxxxxx> wrote: > >> This function forms a sha1 as "xx/yyyy...", but skips over > >> the slot for the slash rather than writing it, leaving it to > >> the caller to do so. It also does not bother to put in a > >> trailing NUL, even though every caller would want it (we're > >> forming a path which by definition is not a directory, so > >> the only thing to do with it is feed it to a system call). > >> > >> Let's make the lives of our callers easier by just writing > >> out the internal "/" and the NUL. > >> ... > > > > I think this makes a lot more sense than making the callers have to do this. > > The cost of fill function having to do the same thing repeatedly is > negligible, so I am OK with the result, but for fairness, this was > not "make the callers do this extra thing", but was "the caller can > prepare these unchanging parts just once, and the fill function that > is repeatedly run does not have to." Yeah, perhaps "does not bother" in the commit message is not entirely fair. But it really does feel like quite a premature optimization to skip the writing of one "/" in the middle of the string, especially as it impacts the interface. -Peff