{"thread":{"id":"65170","subject":"[PATCH v1 0/2] avoid unnecessary strbuf_split*() and strbuf-by-value usage","startedAt":"2026-03-08T18:04:11Z","lastAt":"2026-03-09T19:26:02Z","messageCount":8,"participants":["Deveshi Dwivedi","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"538206","messageId":"20260308180359.31188-1-deveshigurgaon@gmail.com","threadId":"65170","inReplyTo":null,"subject":"[PATCH v1 0/2] avoid unnecessary strbuf_split*() and strbuf-by-value usage","fromName":"Deveshi Dwivedi","fromEmail":"deveshigurgaon@gmail.com","sentAt":"2026-03-08T18:03:57Z","receivedAt":"2026-03-08T18:04:11Z","isPatch":true,"sender":{"key":"deveshigurgaon@gmail.com","avatar":"https://avatars.githubusercontent.com/u/120312681?v=4"},"body":"Junio's \"do not overuse strbuf_split*()\" series [1] calls out\nremaining uses of strbuf_split*() as leftover bits for others to\ncontinue. This series picks up two of them.\n\nwrite_worktree_linking_files() takes two struct strbuf parameters by\nvalue even though it only needs plain path strings. This is the same\nclass of problem fixed for builtin/clean.c in that series.\n\nparse_combine_filter() in list-objects-filter-options.c uses\nstrbuf_split_str() to split a combine: filter spec at '+', then\nmutates each non-final piece to strip the trailing delimiter. An\narray of strbufs is unnecessary; the function processes one sub-spec\nat a time and does not use strbuf editing on the pieces.\n\n[1]: https://lore.kernel.org/git/20250731225433.4028872-1-gitster@pobox.com/\n\nDeveshi Dwivedi (2):\n  worktree: do not pass strbuf by value\n  list-objects-filter-options: avoid strbuf_split_str()\n\n builtin/worktree.c            |  2 +-\n list-objects-filter-options.c | 40 +++++++++++++++++------------------\n worktree.c                    | 22 +++++++++----------\n worktree.h                    |  2 +-\n 4 files changed, 33 insertions(+), 33 deletions(-)\n\n-- \n2.52.0.230.gd8af7cadaa\n\n"},{"id":"538207","messageId":"20260308180359.31188-2-deveshigurgaon@gmail.com","threadId":"65170","inReplyTo":"20260308180359.31188-1-deveshigurgaon@gmail.com","subject":"[PATCH v1 1/2] worktree: do not pass strbuf by value","fromName":"Deveshi Dwivedi","fromEmail":"deveshigurgaon@gmail.com","sentAt":"2026-03-08T18:03:58Z","receivedAt":"2026-03-08T18:04:16Z","isPatch":true,"sender":{"key":"deveshigurgaon@gmail.com","avatar":"https://avatars.githubusercontent.com/u/120312681?v=4"},"body":"write_worktree_linking_files() takes two struct strbuf parameters by\nvalue, even though it only reads path strings from them.\n\nPassing a strbuf by value is misleading and dangerous. The structure\ncarries a pointer to its underlying character array; caller and callee\nend up sharing that storage.  If the callee ever causes the strbuf to\nbe reallocated, the caller's copy becomes a dangling pointer, which\nresults in a double-free when the caller does strbuf_release().\n\nThe function only needs the string values, not the strbuf machinery.\nSwitch it to take const char * and update all callers to pass .buf.\n\nSigned-off-by: Deveshi Dwivedi <deveshigurgaon@gmail.com>\n---\n builtin/worktree.c |  2 +-\n worktree.c         | 22 +++++++++++-----------\n worktree.h         |  2 +-\n 3 files changed, 13 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex bc2d0d645b..4035b1cb06 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -539,7 +539,7 @@ static int add_worktree(const char *path, const char *refname,\n \n \tstrbuf_reset(&sb);\n \tstrbuf_addf(&sb, \"%s/gitdir\", sb_repo.buf);\n-\twrite_worktree_linking_files(sb_git, sb, opts->relative_paths);\n+\twrite_worktree_linking_files(sb_git.buf, sb.buf, opts->relative_paths);\n \tstrbuf_reset(&sb);\n \tstrbuf_addf(&sb, \"%s/commondir\", sb_repo.buf);\n \twrite_file(sb.buf, \"../..\");\ndiff --git a/worktree.c b/worktree.c\nindex 6e2f0f7828..7eba12c6ed 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -445,7 +445,7 @@ void update_worktree_location(struct worktree *wt, const char *path_,\n \tstrbuf_realpath(&path, path_, 1);\n \tstrbuf_addf(&dotgit, \"%s/.git\", path.buf);\n \tif (fspathcmp(wt->path, path.buf)) {\n-\t\twrite_worktree_linking_files(dotgit, gitdir, use_relative_paths);\n+\t\twrite_worktree_linking_files(dotgit.buf, gitdir.buf, use_relative_paths);\n \n \t\tfree(wt->path);\n \t\twt->path = strbuf_detach(&path, NULL);\n@@ -684,7 +684,7 @@ static void repair_gitfile(struct worktree *wt,\n \n \tif (repair) {\n \t\tfn(0, wt->path, repair, cb_data);\n-\t\twrite_worktree_linking_files(dotgit, gitdir, use_relative_paths);\n+\t\twrite_worktree_linking_files(dotgit.buf, gitdir.buf, use_relative_paths);\n \t}\n \n done:\n@@ -742,7 +742,7 @@ void repair_worktree_after_gitdir_move(struct worktree *wt, const char *old_path\n \tif (!file_exists(dotgit.buf))\n \t\tgoto done;\n \n-\twrite_worktree_linking_files(dotgit, gitdir, is_relative_path);\n+\twrite_worktree_linking_files(dotgit.buf, gitdir.buf, is_relative_path);\n done:\n \tstrbuf_release(&gitdir);\n \tstrbuf_release(&dotgit);\n@@ -913,7 +913,7 @@ void repair_worktree_at_path(const char *path,\n \n \tif (repair) {\n \t\tfn(0, gitdir.buf, repair, cb_data);\n-\t\twrite_worktree_linking_files(dotgit, gitdir, use_relative_paths);\n+\t\twrite_worktree_linking_files(dotgit.buf, gitdir.buf, use_relative_paths);\n \t}\n done:\n \tfree(dotgit_contents);\n@@ -1087,17 +1087,17 @@ int init_worktree_config(struct repository *r)\n \treturn res;\n }\n \n-void write_worktree_linking_files(struct strbuf dotgit, struct strbuf gitdir,\n+void write_worktree_linking_files(const char *dotgit, const char *gitdir,\n \t\t\t\t  int use_relative_paths)\n {\n \tstruct strbuf path = STRBUF_INIT;\n \tstruct strbuf repo = STRBUF_INIT;\n \tstruct strbuf tmp = STRBUF_INIT;\n \n-\tstrbuf_addbuf(&path, &dotgit);\n+\tstrbuf_addstr(&path, dotgit);\n \tstrbuf_strip_suffix(&path, \"/.git\");\n \tstrbuf_realpath(&path, path.buf, 1);\n-\tstrbuf_addbuf(&repo, &gitdir);\n+\tstrbuf_addstr(&repo, gitdir);\n \tstrbuf_strip_suffix(&repo, \"/gitdir\");\n \tstrbuf_realpath(&repo, repo.buf, 1);\n \n@@ -1110,11 +1110,11 @@ void write_worktree_linking_files(struct strbuf dotgit, struct strbuf gitdir,\n \t}\n \n \tif (use_relative_paths) {\n-\t\twrite_file(gitdir.buf, \"%s/.git\", relative_path(path.buf, repo.buf, &tmp));\n-\t\twrite_file(dotgit.buf, \"gitdir: %s\", relative_path(repo.buf, path.buf, &tmp));\n+\t\twrite_file(gitdir, \"%s/.git\", relative_path(path.buf, repo.buf, &tmp));\n+\t\twrite_file(dotgit, \"gitdir: %s\", relative_path(repo.buf, path.buf, &tmp));\n \t} else {\n-\t\twrite_file(gitdir.buf, \"%s/.git\", path.buf);\n-\t\twrite_file(dotgit.buf, \"gitdir: %s\", repo.buf);\n+\t\twrite_file(gitdir, \"%s/.git\", path.buf);\n+\t\twrite_file(dotgit, \"gitdir: %s\", repo.buf);\n \t}\n \n \tstrbuf_release(&path);\ndiff --git a/worktree.h b/worktree.h\nindex 06efe26b83..f4e46be385 100644\n--- a/worktree.h\n+++ b/worktree.h\n@@ -240,7 +240,7 @@ int init_worktree_config(struct repository *r);\n  *  dotgit: \"/path/to/foo/.git\"\n  *  gitdir: \"/path/to/repo/worktrees/foo/gitdir\"\n  */\n-void write_worktree_linking_files(struct strbuf dotgit, struct strbuf gitdir,\n+void write_worktree_linking_files(const char *dotgit, const char *gitdir,\n \t\t\t\t  int use_relative_paths);\n \n #endif\n-- \n2.52.0.230.gd8af7cadaa\n\n"},{"id":"538208","messageId":"20260308180359.31188-3-deveshigurgaon@gmail.com","threadId":"65170","inReplyTo":"20260308180359.31188-1-deveshigurgaon@gmail.com","subject":"[PATCH v1 2/2] list-objects-filter-options: avoid strbuf_split_str()","fromName":"Deveshi Dwivedi","fromEmail":"deveshigurgaon@gmail.com","sentAt":"2026-03-08T18:03:59Z","receivedAt":"2026-03-08T18:04:20Z","isPatch":true,"sender":{"key":"deveshigurgaon@gmail.com","avatar":"https://avatars.githubusercontent.com/u/120312681?v=4"},"body":"parse_combine_filter() splits a combine: filter spec at '+' using\nstrbuf_split_str(), which yields an array of strbufs with the\ndelimiter left at the end of each non-final piece.  The code then\nmutates each non-final piece to strip the trailing '+' before parsing.\n\nAllocating an array of strbufs is unnecessary.  The function processes\none sub-spec at a time and does not use strbuf editing on the pieces.\nThe two helpers it calls, has_reserved_character() and\nparse_combine_subfilter(), only read the string content of the strbuf\nthey receive.\n\nWalk the input string directly with strchr() to find each '+'.  Copy\neach sub-spec into a temporary buffer and strip the '+' only when\nanother sub-spec follows.  Change the helpers to take const char *\ninstead of struct strbuf *.\n\nSigned-off-by: Deveshi Dwivedi <deveshigurgaon@gmail.com>\n---\n list-objects-filter-options.c | 40 +++++++++++++++++------------------\n 1 file changed, 20 insertions(+), 20 deletions(-)\n\ndiff --git a/list-objects-filter-options.c b/list-objects-filter-options.c\nindex 7f3e7b8f50..f536085a7c 100644\n--- a/list-objects-filter-options.c\n+++ b/list-objects-filter-options.c\n@@ -125,9 +125,9 @@ int gently_parse_list_objects_filter(\n static const char *RESERVED_NON_WS = \"~`!@#$^&*()[]{}\\\\;'\\\",<>?\";\n \n static int has_reserved_character(\n-\tstruct strbuf *sub_spec, struct strbuf *errbuf)\n+\tconst char *sub_spec, struct strbuf *errbuf)\n {\n-\tconst char *c = sub_spec->buf;\n+\tconst char *c = sub_spec;\n \twhile (*c) {\n \t\tif (*c <= ' ' || strchr(RESERVED_NON_WS, *c)) {\n \t\t\tstrbuf_addf(\n@@ -144,7 +144,7 @@ static int has_reserved_character(\n \n static int parse_combine_subfilter(\n \tstruct list_objects_filter_options *filter_options,\n-\tstruct strbuf *subspec,\n+\tconst char *subspec,\n \tstruct strbuf *errbuf)\n {\n \tsize_t new_index = filter_options->sub_nr;\n@@ -155,7 +155,7 @@ static int parse_combine_subfilter(\n \t\t      filter_options->sub_alloc);\n \tlist_objects_filter_init(&filter_options->sub[new_index]);\n \n-\tdecoded = url_percent_decode(subspec->buf);\n+\tdecoded = url_percent_decode(subspec);\n \n \tresult = has_reserved_character(subspec, errbuf);\n \tif (result)\n@@ -182,34 +182,34 @@ static int parse_combine_filter(\n \tconst char *arg,\n \tstruct strbuf *errbuf)\n {\n-\tstruct strbuf **subspecs = strbuf_split_str(arg, '+', 0);\n-\tsize_t sub;\n+\tconst char *p = arg;\n \tint result = 0;\n \n-\tif (!subspecs[0]) {\n+\tif (!*p) {\n \t\tstrbuf_addstr(errbuf, _(\"expected something after combine:\"));\n \t\tresult = 1;\n \t\tgoto cleanup;\n \t}\n \n-\tfor (sub = 0; subspecs[sub] && !result; sub++) {\n-\t\tif (subspecs[sub + 1]) {\n-\t\t\t/*\n-\t\t\t * This is not the last subspec. Remove trailing \"+\" so\n-\t\t\t * we can parse it.\n-\t\t\t */\n-\t\t\tsize_t last = subspecs[sub]->len - 1;\n-\t\t\tassert(subspecs[sub]->buf[last] == '+');\n-\t\t\tstrbuf_remove(subspecs[sub], last, 1);\n-\t\t}\n-\t\tresult = parse_combine_subfilter(\n-\t\t\tfilter_options, subspecs[sub], errbuf);\n+\twhile (*p && !result) {\n+\t\tconst char *sep = strchr(p, '+');\n+\t\tsize_t len = sep ? (size_t)(sep - p + 1) : strlen(p);\n+\t\tchar *sub = xmemdupz(p, len);\n+\n+\t\t/* strip '+' separator, but only when more sub-specs follow */\n+\t\tif (sep && *(sep + 1))\n+\t\t\tsub[len - 1] = '\\0';\n+\n+\t\tresult = parse_combine_subfilter(filter_options, sub, errbuf);\n+\t\tfree(sub);\n+\t\tif (!sep)\n+\t\t\tbreak;\n+\t\tp = sep + 1;\n \t}\n \n \tfilter_options->choice = LOFC_COMBINE;\n \n cleanup:\n-\tstrbuf_list_free(subspecs);\n \tif (result)\n \t\tlist_objects_filter_release(filter_options);\n \treturn result;\n-- \n2.52.0.230.gd8af7cadaa\n\n"},{"id":"538280","messageId":"xmqqh5qp6oun.fsf@gitster.g","threadId":"65170","inReplyTo":"20260308180359.31188-2-deveshigurgaon@gmail.com","subject":"Re: [PATCH v1 1/2] worktree: do not pass strbuf by value","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-09T14:48:16Z","receivedAt":"2026-03-09T14:48:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Deveshi Dwivedi <deveshigurgaon@gmail.com> writes:\n\n> The function only needs the string values, not the strbuf machinery.\n> Switch it to take const char * and update all callers to pass .buf.\n\nMakes perfect sense.  Thanks for noticing and fixing these.\n\n> -\t\twrite_worktree_linking_files(dotgit, gitdir, use_relative_paths);\n> +\t\twrite_worktree_linking_files(dotgit.buf, gitdir.buf, use_relative_paths);\n> ...\n> -void write_worktree_linking_files(struct strbuf dotgit, struct strbuf gitdir,\n> +void write_worktree_linking_files(const char *dotgit, const char *gitdir,\n>  \t\t\t\t  int use_relative_paths)\n>  {\n\nThis updated function signature makes it plenty clear that the\nfunction does not modify anything in gitdir, so the caller shouldn't\nbe affected.  Nice.\n"},{"id":"538292","messageId":"xmqqjyvl57yv.fsf@gitster.g","threadId":"65170","inReplyTo":"20260308180359.31188-3-deveshigurgaon@gmail.com","subject":"Re: [PATCH v1 2/2] list-objects-filter-options: avoid strbuf_split_str()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-09T15:38:16Z","receivedAt":"2026-03-09T15:38:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Deveshi Dwivedi <deveshigurgaon@gmail.com> writes:\n\n> parse_combine_filter() splits a combine: filter spec at '+' using\n> strbuf_split_str(), which yields an array of strbufs with the\n> delimiter left at the end of each non-final piece.  The code then\n> mutates each non-final piece to strip the trailing '+' before parsing.\n>\n> Allocating an array of strbufs is unnecessary.  The function processes\n> one sub-spec at a time and does not use strbuf editing on the pieces.\n> The two helpers it calls, has_reserved_character() and\n> parse_combine_subfilter(), only read the string content of the strbuf\n> they receive.\n>\n> Walk the input string directly with strchr() to find each '+'.  Copy\n> each sub-spec into a temporary buffer and strip the '+' only when\n> another sub-spec follows.  Change the helpers to take const char *\n> instead of struct strbuf *.\n\nMakes sense.  Instead of finding '+' and making many small copies\npiecemeal, you could make a single copy of \"const char *arg\" once,\nwalk that string using strchr() looking for the next '+', and\nreplace '+' with '\\0' before processing the current piece and\niterate, which may reduce the need for many small allocations and\ndeallocations, but I do not know if it is worth it.  Benchmarking\nit would not yield measurable difference, I suspect.\n\n> +\twhile (*p && !result) {\n> +\t\tconst char *sep = strchr(p, '+');\n> +\t\tsize_t len = sep ? (size_t)(sep - p + 1) : strlen(p);\n> +\t\tchar *sub = xmemdupz(p, len);\n> +\n> +\t\t/* strip '+' separator, but only when more sub-specs follow */\n> +\t\tif (sep && *(sep + 1))\n> +\t\t\tsub[len - 1] = '\\0';\n> +\n> +\t\tresult = parse_combine_subfilter(filter_options, sub, errbuf);\n> +\t\tfree(sub);\n> +\t\tif (!sep)\n> +\t\t\tbreak;\n> +\t\tp = sep + 1;\n>  \t}\n\nHmph, would this loop handle a trailing '+' the same way as before,\ne.g., \"combine:tree:2+\"?  The original would have split the string\ninto [\"tree:2+\", \"\"] and the last call to parse_combine_subfilter()\nwould have been made with an empty string.  The new code does not\nmake that last call with an empty string.  Perhaps the differences\ndo not matter?  I dunno.\n\nOther than that, nice to see one fewer use of \"splitting into an\narray of strbuf\" pattern.\n\nThanks.\n\n\n\n"},{"id":"538309","messageId":"20260309190112.GA309867@coredump.intra.peff.net","threadId":"65170","inReplyTo":"xmqqjyvl57yv.fsf@gitster.g","subject":"Re: [PATCH v1 2/2] list-objects-filter-options: avoid strbuf_split_str()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-09T19:01:12Z","receivedAt":"2026-03-09T19:01:20Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 09, 2026 at 08:38:16AM -0700, Junio C Hamano wrote:\n\n> > +\twhile (*p && !result) {\n> > +\t\tconst char *sep = strchr(p, '+');\n> > +\t\tsize_t len = sep ? (size_t)(sep - p + 1) : strlen(p);\n> > +\t\tchar *sub = xmemdupz(p, len);\n> > +\n> > +\t\t/* strip '+' separator, but only when more sub-specs follow */\n> > +\t\tif (sep && *(sep + 1))\n> > +\t\t\tsub[len - 1] = '\\0';\n> > +\n> > +\t\tresult = parse_combine_subfilter(filter_options, sub, errbuf);\n> > +\t\tfree(sub);\n> > +\t\tif (!sep)\n> > +\t\t\tbreak;\n> > +\t\tp = sep + 1;\n> >  \t}\n> \n> Hmph, would this loop handle a trailing '+' the same way as before,\n> e.g., \"combine:tree:2+\"?  The original would have split the string\n> into [\"tree:2+\", \"\"] and the last call to parse_combine_subfilter()\n> would have been made with an empty string.  The new code does not\n> make that last call with an empty string.  Perhaps the differences\n> do not matter?  I dunno.\n\nI think the original was wrong in its parsing. The first entry should be\n\"tree:2\", without the trailing \"+\". And that would almost always result\nin rejecting the string, because the \"+\" doesn't make any sense for most\nfilters. The exception is:\n\n  blob=$(echo foo | git hash-object -w --stdin)\n  git tag foo+ $blob\n  git rev-list --filter=combine:sparse:oid=foo+\n\nwhich happens to work. But it is wrong according to the documentation,\nwhich says that \"+\" needs to be escaped if you want it passed along to\nthe sub-filter.\n\nSo flagging a trailing \"+\" as an error would probably be OK, and match\nwhat happens now. But quietly ignoring it is perhaps friendlier (or less\nfriendly, if you think it might let a typo'd input go unnoticed).\n\n-Peff\n"},{"id":"538311","messageId":"20260309190823.GB309867@coredump.intra.peff.net","threadId":"65170","inReplyTo":"20260308180359.31188-3-deveshigurgaon@gmail.com","subject":"Re: [PATCH v1 2/2] list-objects-filter-options: avoid strbuf_split_str()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-09T19:08:23Z","receivedAt":"2026-03-09T19:08:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Mar 08, 2026 at 06:03:59PM +0000, Deveshi Dwivedi wrote:\n\n> +\twhile (*p && !result) {\n> +\t\tconst char *sep = strchr(p, '+');\n> +\t\tsize_t len = sep ? (size_t)(sep - p + 1) : strlen(p);\n> +\t\tchar *sub = xmemdupz(p, len);\n\nThe cast to size_t made me look twice to see if something tricky was\ngoing on. We don't usually bother explicitly casting from a ptrdiff_t\ninto a size_t.\n\nHowever, might this all be simpler with strchrnul? Something like:\n\n  const char *end = strchrnul(p, '+');\n  char *sub = xmemdupz(p, end - p);\n\n  ...parse sub...\n\n  if (!*end)\n\tbreak; /* found NUL at end of string */\n  p = end + 1;\n\nNotice I cut off the \"+\" when we find it, because I think...\n\n> +\t\t/* strip '+' separator, but only when more sub-specs follow */\n> +\t\tif (sep && *(sep + 1))\n> +\t\t\tsub[len - 1] = '\\0';\n\n...this is wrong. I know you are matching what the current code does,\nbut it does not match the documentation, and does not actually make any\nsense in practice.\n\n\nOther than that, this looks nice, and I am happy to see more\nstrbuf_split() calls going away.\n\nI think you could in theory drop the xmemdupz() here, too, and feed the\nptr/len combo into parse_combine_subfilter(), which then percent-decodes\ninto a newly allocated buffer. But it is probably not worth trying to\nsqueeze out one extra allocation here. It is not like people have huge\nlists of combined filters; we'd expect to see a couple at most.\n\n-Peff\n"},{"id":"538312","messageId":"20260309192600.GC309867@coredump.intra.peff.net","threadId":"65170","inReplyTo":"20260308180359.31188-2-deveshigurgaon@gmail.com","subject":"coccinelle to catch pass-by-value?, was: [PATCH v1 1/2] worktree: do not pass strbuf by value","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-09T19:26:00Z","receivedAt":"2026-03-09T19:26:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Mar 08, 2026 at 06:03:58PM +0000, Deveshi Dwivedi wrote:\n\n> The function only needs the string values, not the strbuf machinery.\n> Switch it to take const char * and update all callers to pass .buf.\n\nNice catch. I wonder if we can get the compiler or other static analysis\nto complain about this mistake. The best I could come up with is:\n\ndiff --git a/contrib/coccinelle/strbuf.cocci b/contrib/coccinelle/strbuf.cocci\nindex 5f06105df6..665f56d070 100644\n--- a/contrib/coccinelle/strbuf.cocci\n+++ b/contrib/coccinelle/strbuf.cocci\n@@ -60,3 +60,10 @@ expression E1, E2;\n @@\n - strbuf_addstr(E1, real_path(E2));\n + strbuf_add_real_path(E1, E2);\n+\n+@@\n+expression F, ARG1, ARG2;\n+struct strbuf SB;\n+@@\n+- F(ARG1, SB, ARG2)\n++ F(ARG1, &SB, ARG2)\n\nIt rewrites a non-pointer argument into a pointer. That's not enough to\nactually make the code work, but it would alert a developer that they\nneeded to follow-through on the rest of it. Or maybe it would just\nconfuse them without further hints.\n\nI think there may be a way to get coccinelle to just emit an error\nmessage describing the situation, but it relies on python extensions,\nwhich I'm not sure we currently require.\n\nAnyway, your patch is obviously good and anything further we do would\nwant to come on top of it.\n\n-Peff\n"}]}