On Mon, Jan 02, 2017 at 05:27:59PM +0100, Michael Haggerty wrote: > > or something. But I doubt the single allocation is breaking the bank, > > and it has the nice side effect that callers can pass in a const string > > (I didn't check yet whether that enables further cleanups). > > The last patch in the series passes ref_update::refname to this > function, which is `const char *`. With your suggested change, either > that member would have to be made non-const, or it would have to be cast > to const at the `try_remove_empty_parents()` callsite. > > Making the member non-const would be easy, though it loses a tiny bit of > documentation and safety against misuse. Also, scribbling even > temporarily over that member would mean that this code is not > thread-safe (though it seems unlikely that we would ever bother making > it multithreaded). > > I think I prefer the current version because it loosens the coupling > between this function and its callers. But I don't mind either way if > somebody feels strongly about it. OK, let's take what you have here, then. > As an aside, I wonder whether we would be discussing this at all if we > had stack-allocated strbufs that could be used without allocating heap > memory in the usual case. I'm not sure. We still pay the memcpy(), though I don't know how substantial that is compared to an allocation. For these small strings, probably not very. -Peff