> On 2015-02-02, at 09:53, Jeff King <peff@xxxxxxxx> wrote: > > I think this is probably the best fix, and is the pattern we use > elsewhere when touching alt->base. > > We _could_ further change this to have for_each_loose_file_in_objdir > actually use alt->base as its scratch buffer, writing the object > filenames into the end of it (i.e., what it was designed for). But: > > 1. We still need a strbuf scratch-buffer for the non-alternate object > directory. So we'd have to push more code there to over-allocate > the buffer, and then for_each_loose_file_in_objdir would assume > we always feed it a buffer with the extra slop. That would work, > but I find the strbuf approach a little safer; there's not an > implicit over-allocation far away in the code preventing us from > overflowing a buffer. > > 2. The reason for the existing alt->base behavior is that the > sha1_file code gets fed objects one at a time, and don't want to > pay strbuf overhead for each. With the iterator, we know we are > going to hit a bunch of objects, so we only have to pay the strbuf > overhead once for the iteration. So there's not the same > performance penalty, and we can stick with the strbuf if we prefer > it. Thanks for your feedback. I considered the same, and came to a similar conclusion. The strbuf cost is only once per alternate, so I feel on balance it's more robust to use alt->base consistently inside each function, rather than have this a more fragile special case to save allocation of only one path. Updated the test patch. Jonathon Mah me@xxxxxxxxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html