Re: [PATCH] clean: do not pass strbuf by value
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
Show 18 quoted lines
> 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?
Show 5 quoted lines
>
> 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
Patrick Steinhardt <ps@pks.im>