]>
Commit | Line | Data |
---|---|---|
bd111141 | 1 | = coccinelle |
dd5d052c | 2 | |
bd111141 GC |
3 | This directory provides Coccinelle (http://coccinelle.lip6.fr/) semantic patches |
4 | that might be useful to developers. | |
5 | ||
6 | == Types of semantic patches | |
dd5d052c SG |
7 | |
8 | * Using the semantic transformation to check for bad patterns in the code; | |
9 | The target 'make coccicheck' is designed to check for these patterns and | |
10 | it is expected that any resulting patch indicates a regression. | |
11 | The patches resulting from 'make coccicheck' are small and infrequent, | |
12 | so once they are found, they can be sent to the mailing list as per usual. | |
13 | ||
14 | Example for introducing new patterns: | |
15 | 67947c34ae (convert "hashcmp() != 0" to "!hasheq()", 2018-08-28) | |
16 | b84c783882 (fsck: s/++i > 1/i++/, 2018-10-24) | |
17 | ||
18 | Example of fixes using this approach: | |
19 | 248f66ed8e (run-command: use strbuf_addstr() for adding a string to | |
20 | a strbuf, 2018-03-25) | |
21 | f919ffebed (Use MOVE_ARRAY, 2018-01-22) | |
22 | ||
23 | These types of semantic patches are usually part of testing, c.f. | |
24 | 0860a7641b (travis-ci: fail if Coccinelle static analysis found something | |
25 | to transform, 2018-07-23) | |
26 | ||
27 | * Using semantic transformations in large scale refactorings throughout | |
28 | the code base. | |
29 | ||
30 | When applying the semantic patch into a real patch, sending it to the | |
31 | mailing list in the usual way, such a patch would be expected to have a | |
32 | lot of textual and semantic conflicts as such large scale refactorings | |
33 | change function signatures that are used widely in the code base. | |
34 | A textual conflict would arise if surrounding code near any call of such | |
35 | function changes. A semantic conflict arises when other patch series in | |
36 | flight introduce calls to such functions. | |
37 | ||
38 | So to aid these large scale refactorings, semantic patches can be used. | |
39 | However we do not want to store them in the same place as the checks for | |
40 | bad patterns, as then automated builds would fail. | |
41 | That is why semantic patches 'contrib/coccinelle/*.pending.cocci' | |
42 | are ignored for checks, and can be applied using 'make coccicheck-pending'. | |
43 | ||
44 | This allows to expose plans of pending large scale refactorings without | |
45 | impacting the bad pattern checks. | |
316e3886 | 46 | |
bd111141 | 47 | == Git-specific tips & things to know about how we run "spatch": |
316e3886 ÆAB |
48 | |
49 | * The "make coccicheck" will piggy-back on | |
50 | "COMPUTE_HEADER_DEPENDENCIES". If you've built a given object file | |
51 | the "coccicheck" target will consider its depednency to decide if | |
52 | it needs to re-run on the corresponding source file. | |
53 | ||
54 | This means that a "make coccicheck" will re-compile object files | |
55 | before running. This might be unexpected, but speeds up the run in | |
56 | the common case, as e.g. a change to "column.h" won't require all | |
57 | coccinelle rules to be re-run against "grep.c" (or another file | |
58 | that happens not to use "column.h"). | |
59 | ||
60 | To disable this behavior use the "SPATCH_USE_O_DEPENDENCIES=NoThanks" | |
61 | flag. | |
d0e624ae ÆAB |
62 | |
63 | * To speed up our rules the "make coccicheck" target will by default | |
64 | concatenate all of the *.cocci files here into an "ALL.cocci", and | |
65 | apply it to each source file. | |
66 | ||
67 | This makes the run faster, as we don't need to run each rule | |
68 | against each source file. See the Makefile for further discussion, | |
69 | this behavior can be disabled with "SPATCH_CONCAT_COCCI=". | |
70 | ||
71 | But since they're concatenated any <id> in the <rulname> (e.g. "@ | |
72 | my_name", v.s. anonymous "@@") needs to be unique across all our | |
73 | *.cocci files. You should only need to name rules if other rules | |
74 | depend on them (currently only one rule is named). | |
6fae3aaf ÆAB |
75 | |
76 | * To speed up incremental runs even more use the "spatchcache" tool | |
77 | in this directory as your "SPATCH". It aimns to be a "ccache" for | |
78 | coccinelle, and piggy-backs on "COMPUTE_HEADER_DEPENDENCIES". | |
79 | ||
80 | It caches in Redis by default, see it source for a how-to. | |
81 | ||
82 | In one setup with a primed cache "make coccicheck" followed by a | |
83 | "make clean && make" takes around 10s to run, but 2m30s with the | |
84 | default of "SPATCH_CONCAT_COCCI=Y". | |
85 | ||
86 | With "SPATCH_CONCAT_COCCI=" the total runtime is around ~6m, sped | |
87 | up to ~1m with "spatchcache". | |
88 | ||
89 | Most of the 10s (or ~1m) being spent on re-running "spatch" on | |
90 | files we couldn't cache, as we didn't compile them (in contrib/* | |
91 | and compat/* mostly). | |
92 | ||
93 | The absolute times will differ for you, but the relative speedup | |
94 | from caching should be on that order. | |
3bd0097c GC |
95 | |
96 | == Authoring and reviewing coccinelle changes | |
97 | ||
98 | * When a .cocci is made, both the Git changes and .cocci file should be | |
99 | reviewed. When reviewing such a change, do your best to understand the .cocci | |
100 | changes (e.g. by asking the author to explain the change) and be explicit | |
101 | about your understanding of the changes. This helps us decide whether input | |
102 | from coccinelle experts is needed or not. If you aren't sure of the cocci | |
103 | changes, indicate what changes you actively endorse and leave an Acked-by | |
104 | (instead of Reviewed-by). | |
105 | ||
106 | * Authors should consider that reviewers may not be coccinelle experts, thus the | |
107 | the .cocci changes may not be self-evident. A plain text description of the | |
108 | changes is strongly encouraged, especially when using more esoteric features | |
109 | of the language. | |
110 | ||
111 | * .cocci rules should target only the problem it is trying to solve; "collateral | |
112 | damage" is not allowed. Reviewers should look out and flag overly-broad rules. | |
113 | ||
114 | * Consider the cost-benefit ratio of .cocci changes. In particular, consider the | |
115 | effect on the runtime of "make coccicheck", and how often your .cocci check | |
116 | will catch something valuable. As a rule of thumb, rules that can bail early | |
117 | if a file doesn't have a particular token will have a small impact on runtime, | |
118 | and vice-versa. | |
119 | ||
120 | * .cocci files used for refactoring should be temporarily kept in-tree to aid | |
121 | the refactoring of out-of-tree code (e.g. in-flight topics). Periodically | |
122 | evaluate the cost-benefit ratio to determine when the file should be removed. | |
123 | For example, consider how many out-of-tree users are left and how much this | |
124 | slows down "make coccicheck". |