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

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 10, 2023, 20:33 UTC
Message-ID: <xmqqwn4pgt1f.fsf@gitster.g>
In-Reply-To: <850c94d5-973a-335e-903f-e0ab7e52f37e@web.de>

René Scharfe <l.s.r@web.de> writes:

Show 15 quoted lines

>>> -				strbuf_grow(tree_path, base_path_len + entry.pathlen + 1);
>>> +				strbuf_grow(tree_path, entry.pathlen + 1);
>>>  				strbuf_add(tree_path, entry.path, entry.pathlen);
>>>  				strbuf_addch(tree_path, '/');
>>>  			}
>>
>> The size_t conversion is trivially correct.
>>
>> But what do you mean with "don't double the[...]"? Do you mean that this
>> manages to evade growing these to 24 etc?
>
> strbuf_setlen() truncates the string to the directory name.  strbuf_grow() then
> makes enough room to add that directory name again (that's what I mean with
> "double") plus the entry path.  We don't add the directory name a second time,
> so we don't need to make room for it.

Yeah, I think I made the same mistake number of years ago, thinking that strbuf_grow() was to grow the buffer to the given size, but in reality it is to grow the buffer by the given size, which felt a bit unnatural, at least to me. 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-<.

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.