Re: [PATCH] clean: do not pass strbuf by value

[Patrick Steinhardt <ps@pks.im>]

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@pobox.com>
> ---
>  builtin/clean.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)

Good finding with an obvious fix. Thanks!

Patrick