Subject: Re: [PATCH] cache-tree: fix strbuf growth in prime_cache_tree_rec()

From: Junio C Hamano <[email-masked]>
Date: Fri, 10 Feb 2023 18:46:06 -0800

Jeff King <[email-masked]> writes:

>> ...  I do not feel it too strongly but we
>> might want to rename _grow() to _grow_by() and make _grow() call it
>> while giving deprecation warning X-<.
>
> Having been confused by that myself, I would be happy to see such a
> name change.

If we did not know how useless explicit growth control is, we would
likely have a pair of helpers, _grow_by() and _grow_to(), but given
that ...

>> There are ~45 calls to strbuf_grow() in C files other than strbuf.c;
>> I suspect probably a half or more of them can and should be removed
>> to reduce the resulting code size without hurting anything.
>
> My gut feeling is that your suspicion is giving strbuf_grow() users too
> much credit. ;) And having looked at the first 7 grep hits, every single
> one of them seemed pointless to me.

... we'd only have a very limited number of callers for which the
helper makes sense, I am not sure if it is even worth the renaming.

Or just rename it to _grow_to() while fixing what it does, as
grow_to() is what programmers would expect naturally?

> I wonder if these would make a good #leftoverbits / micro-project
> candidate.

The task is to pick one or two from these 45 hits, analyze what
would happen if we remove the _grow() calls.  For many of them, the
result of such analysis would say the calls are pointless, but for
some, there hopefully are solid reasons why explicit sizing makes
sense.  The former will be just removed, while the latter will be
kept with a new in-code comment to record why it is worth having the
call.  The parameter may need to be updated for them at the same
time.

It can be done extremely poorly without breaking anything in the
test suite, or it can be done expertly.  Unless the result are
reviewed competently, it may make a rather poor micro-project
experience.

So, I dunno.