From: Junio C Hamano Date: Fri, 10 Feb 2023 20:33:00 GMT Subject: Re: [PATCH] cache-tree: fix strbuf growth in prime_cache_tree_rec() Message-ID: In-Reply-To: <850c94d5-973a-335e-903f-e0ab7e52f37e@web.de> René Scharfe writes: >>> - 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.