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

Reply via email to