On Tue, Oct 04, 2016 at 02:46:44PM -0700, Junio C Hamano wrote:

> Jacob Keller <jacob.kel...@gmail.com> writes:
> 
> > On Mon, Oct 3, 2016 at 1:35 PM, Jeff King <p...@peff.net> 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

Reply via email to