While reviewing the write_worktree_linking_files() fix [1], Jeff King suggested adding a coccinelle rule to detect functions that take struct strbuf by value. I previously posted an RFC discussing such a rule and its implementation [2].

Patch 1/2 adds a coccinelle rule to detect functions that take struct strbuf by value and rewrites the parameter to a pointer to highlight the issue.

Patch 2/2 fixes the one remaining instance found by the rule in stash.c by changing the parameter to struct strbuf * and updating the caller accordingly.

The worktree.c instance that motivated the rule is already fixed by [1], so only the stash.c case remains.

[1] https://lore.kernel.org/git/20260309192600.GC309867@coredump.intra.peff.net/ [2] https://lore.kernel.org/git/CAG7UgESKLMnO_4+PSJUt-TXJxFQyxEEfpCmJfMmTw2+rhT-HWw@mail.gmail.com/

Deveshi Dwivedi (2):
  coccinelle: detect struct strbuf passed by value
  stash: do not pass strbuf by value

 builtin/stash.c                 |  6 +++---
 contrib/coccinelle/strbuf.cocci | 11 +++++++++++
 2 files changed, 14 insertions(+), 3 deletions(-)

Passing a struct strbuf by value to a function copies the struct but shares the underlying character array between caller and callee. If the callee causes a reallocation, the caller's copy becomes a dangling pointer, leading to a double-free when strbuf_release() is called. There is no coccinelle rule to catch this pattern.

Jeff King suggested adding one during review of the write_worktree_linking_files() fix [1], and noted that a reporting rule using coccinelle's Python scripting extensions could emit a descriptive warning, but we do not currently require Python support in coccinelle.

Add a transformation rule that rewrites a by-value strbuf parameter to a pointer. The detection is identical to what a Python-based reporting rule would catch; only the presentation differs. The resulting diff will not produce compilable code on its own (callers and the function body still need updating), but the spatch output alerts the developer that the signature needs attention. This is consistent with the other rules in strbuf.cocci, which also rewrite to the preferred form.

[1] https://lore.kernel.org/git/20260309192600.GC309867@coredump.intra.peff.net/

Signed-off-by: Deveshi Dwivedi <deveshigurgaon@gmail.com>
---
 contrib/coccinelle/strbuf.cocci | 11 +++++++++++
 1 file changed, 11 insertions(+)

save_untracked_files() takes its 'files' parameter as struct strbuf by value. Passing a strbuf by value copies the struct but shares the underlying buffer between caller and callee, risking a dangling pointer and double-free if the callee reallocates.

The function needs both the buffer and its length for pipe_command(), so a plain const char * is not sufficient here. Switch the parameter to struct strbuf * and update the caller to pass a pointer.

Signed-off-by: Deveshi Dwivedi <deveshigurgaon@gmail.com>
---
 builtin/stash.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Deveshi Dwivedi <deveshigurgaon@gmail.com> writes:

Nicely done.  Will queue.  Thanks.