# [RFC] coccinelle: detect struct strbuf passed by value 3 messages from 2026-03-14 to 2026-03-15. Participants: Deveshi Dwivedi, Jeff King. Thread: https://gitlist.dev/t/65246 ## Deveshi Dwivedi, 2026-03-14 17:12 Subject: [RFC] coccinelle: detect struct strbuf passed by value Message-ID: URL: https://gitlist.dev/e/CAG7UgESKLMnO_4%2BPSJUt-TXJxFQyxEEfpCmJfMmTw2%2BrhT-HWw%40mail.gmail.com ``` While reviewing the write_worktree_linking_files() fix [1], Jeff King suggested adding a coccinelle rule to catch functions that take struct strbuf by value. He noted that a reporting rule using coccinelle's Python scripting extensions could emit a descriptive warning. A transformation rule achieves the same detection without the dependency. It rewrites a by-value strbuf parameter to a pointer. 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. The rule itself: @@ identifier fn, param; @@ fn(..., - struct strbuf param + struct strbuf *param ,...) { ... } Running 'make coccicheck COCCI=contrib/coccinelle/strbuf.cocci' on master catches two instances: - write_worktree_linking_files() in worktree.c, which is already fixed by the series in [1]. - save_untracked_files() in builtin/stash.c, which takes 'struct strbuf files' by value. This is the same class of bug. Sending this as an RFC to get feedback on whether this rule would be a reasonable addition before preparing a patch. [1] https://lore.kernel.org/git/20260309192600.GC309867@coredump.intra.peff.net/ Deveshi Dwivedi ``` ## Jeff King, 2026-03-15 02:55 Subject: Re: [RFC] coccinelle: detect struct strbuf passed by value Message-ID: <20260315025508.GA926820@coredump.intra.peff.net> URL: https://gitlist.dev/e/20260315025508.GA926820%40coredump.intra.peff.net In-Reply-To: ``` On Sat, Mar 14, 2026 at 10:42:19PM +0530, Deveshi Dwivedi wrote: > A transformation rule achieves the same detection without the > dependency. It rewrites a by-value strbuf parameter to a pointer. > 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. > > The rule itself: > > @@ > identifier fn, param; > @@ > fn(..., > - struct strbuf param > + struct strbuf *param > ,...) > { > ... > } This is much better than what I posted before. The real source of the problem is the functions which take strbufs by value, not the callsites that pass it to them (and mine was checking the latter). And your use of "..." is better than what I had. I think mine insisted on having arguments after the strbuf, which is why it failed to find the case in save_untracked_files(). So the only question to me is whether people who hit the coccinelle suggestion might be confused by the patch output, since it doesn't carry any rationale. But I would rather catch the problem and risk confusion then have it go unnoticed. -Peff ``` ## Deveshi Dwivedi, 2026-03-15 07:29 Subject: Re: [RFC] coccinelle: detect struct strbuf passed by value Message-ID: URL: https://gitlist.dev/e/CAG7UgETB7uPvWuZL08i48JvVgefJAoUqjt96C0dPNV_Qpj8jfw%40mail.gmail.com In-Reply-To: <20260315025508.GA926820@coredump.intra.peff.net> ``` > So the only question to me is whether people who hit the coccinelle > suggestion might be confused by the patch output, since it doesn't carry > any rationale. But I would rather catch the problem and risk confusion > then have it go unnoticed. > > -Peff Thanks for the review. I agree that the patch output may be a bit confusing, but it still seems useful to flag these cases. I will send a follow-up patch adding the rule and fixing the stash case. Thanks, Deveshi ```