On Tue, Jul 29, 2025 at 02:03:27PM -0700, Junio C Hamano wrote: > When you pass a structure by value, the callee can modify the > contents of the structure that was passed in without having to worry > about changing the structure the caller has. Passing structure by s/structure/structures/ > value sometimes (but not very often) can be a valid way to give > callee a temporary variable it can freely modify. > > But not a structure with members that are pointers, like a strbuf. > > builtin/clean.c:list_and_choose() reads a line interactively from > the user, and passes the line (in a strbuf) to parse_choice() by > value, which then munges by replacing ',' with ' ' (to accept both > comma and space separated list of choices). But because the strbuf > passed by value still shares the underlying character array buf[], > this ends up munging the caller's strbuf contents. > > This is a catastrophe waiting to happen. If the callee causes the > strbuf to be reallocated, the buf[] the caller has will become > dangling, and when the caller does strbuf_release(), it would result > in double-free. > > Stop calling the function with misleading call-by-value with strbuf. I think the second "with" should be dropped? > > Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx> > --- > builtin/clean.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) Good finding with an obvious fix. Thanks! Patrick
