On Mon, Oct 17 2022, Jeff King wrote: > On Fri, Oct 14, 2022 at 05:31:16PM +0200, Ævar Arnfjörð Bjarmason wrote: > >> The big change in this belated v3 is that we now run against a >> concatenated version of all our *.cocci files. This is something I >> discussed with Jeff King at Git Merge, after having confirmed the >> viability of that approach on the cocci mailing list. > > Is that safe? The last time it was brought up on this list (AFAIK) was > in: > > https://lore.kernel.org/git/alpine.DEB.2.20.1808030755350.2446@hadrien/ > > where Julia said: > > I'm surprised that the above cat command would work. Semantic patch rules > have names, and Coccinelle will not be happy isf two rules have the same > name. Some may also have variables declared in initializers, although > perhaps the ones in the kernel don't do this. Causing these variables to > be shared would not have a good effect. > > Putting everything together can also go counter to the optimizations that > Coccinelle provides. [...] > > Maybe we don't do any of the things that could trigger problems in our > spatch rules. But it's not clear to me what we're risking. Do you have a > link for further discussion? I think 10/11's commit message should answer your question: https://lore.kernel.org/git/patch-v3-10.11-52177ea2a68-20221014T152553Z-avarab@xxxxxxxxx/ The tl;dr is that it's not safe in the general case, as noted in the post you & the more recent one I linked to in 10/11. So, with this series doing: perl -pi -e 's/swap/preincrement/g' contrib/coccinelle/swap.cocci Will error it if you run it with "SPATCH_CONCAT_COCCI=Y", but not with "SPATCH_CONCAT_COCCI=", as the rule names conflict in the ALL.cocci. But as 10/11 notes we can just avoid this by not picking conflicting names, which doesn't seem like an undue burden. AFAICT we have 5 named rules, and seemingly only 1/4 actually needs its name, the rest are apparently only using it for self-documentation, and we could either remove the name, or accomplish the same with a comment: diff --git a/contrib/coccinelle/hashmap.cocci b/contrib/coccinelle/hashmap.cocci index d69e120ccff..c5dbb4557b5 100644 --- a/contrib/coccinelle/hashmap.cocci +++ b/contrib/coccinelle/hashmap.cocci @@ -1,4 +1,4 @@ -@ hashmap_entry_init_usage @ +@@ expression E; struct hashmap_entry HME; @@ diff --git a/contrib/coccinelle/preincr.cocci b/contrib/coccinelle/preincr.cocci index 7fe1e8d2d9a..ae42cb07302 100644 --- a/contrib/coccinelle/preincr.cocci +++ b/contrib/coccinelle/preincr.cocci @@ -1,4 +1,4 @@ -@ preincrement @ +@@ identifier i; @@ - ++i > 1 diff --git a/contrib/coccinelle/strbuf.cocci b/contrib/coccinelle/strbuf.cocci index 0970d98ad72..5f06105df6d 100644 --- a/contrib/coccinelle/strbuf.cocci +++ b/contrib/coccinelle/strbuf.cocci @@ -1,4 +1,4 @@ -@ strbuf_addf_with_format_only @ +@@ expression E; constant fmt !~ "%"; @@ diff --git a/contrib/coccinelle/swap.cocci b/contrib/coccinelle/swap.cocci index a0934d1fdaf..522177afb66 100644 --- a/contrib/coccinelle/swap.cocci +++ b/contrib/coccinelle/swap.cocci @@ -1,4 +1,4 @@ -@ swap_with_declaration @ +@@ type T; identifier tmp; T a, b;
