diff options
| author | Nguyễn Thái Ngọc Duy <pclouds@gmail.com> | 2019-04-16 09:33:18 (GMT) |
|---|---|---|
| committer | Junio C Hamano <gitster@pobox.com> | 2019-04-16 09:56:51 (GMT) |
| commit | a133c40b23c80ed77cfe077213a45af67be28f74 (patch) | |
| tree | cbcaf6cee486fcae3c378f44e0d3a647b108374a /contrib | |
| parent | 7fdff47432bbb591b6e44ebab48e1f206521cd1b (diff) | |
| download | git-a133c40b23c80ed77cfe077213a45af67be28f74.zip git-a133c40b23c80ed77cfe077213a45af67be28f74.tar.gz git-a133c40b23c80ed77cfe077213a45af67be28f74.tar.bz2 | |
commit.cocci: refactor code, avoid double rewrite
"maybe" pointer in 'struct commit' is tricky because it can be lazily
initialized to take advantage of commit-graph if available. This makes
it not safe to access directly.
This leads to a rule in commit.cocci to rewrite 'x->maybe_tree' to
'get_commit_tree(x)'. But that rule alone could lead to incorrectly
rewrite assignments, e.g. from
x->maybe_tree = yes
to
get_commit_tree(x) = yes
Because of this we have a second rule to revert this effect. Szeder
found out that we could do better by performing the assignment rewrite
rule first, then the remaining is read-only access and handled by the
current first rule.
For this to work, we need to transform "x->maybe_tree = y" to something
that does NOT contain "x->maybe_tree" to avoid the original first
rule. This is where set_commit_tree() comes in.
Helped-by: SZEDER Gábor <szeder.dev@gmail.com>
Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to 'contrib')
| -rw-r--r-- | contrib/coccinelle/commit.cocci | 20 |
1 files changed, 13 insertions, 7 deletions
diff --git a/contrib/coccinelle/commit.cocci b/contrib/coccinelle/commit.cocci index c49aa55..663658a 100644 --- a/contrib/coccinelle/commit.cocci +++ b/contrib/coccinelle/commit.cocci @@ -10,19 +10,25 @@ expression c; - c->maybe_tree->object.oid.hash + get_commit_tree_oid(c)->hash -// These excluded functions must access c->maybe_tree direcly. @@ -identifier f !~ "^(get_commit_tree|get_commit_tree_in_graph_one|load_tree_for_commit)$"; +identifier f !~ "^set_commit_tree$"; expression c; +expression s; @@ f(...) {<... -- c->maybe_tree -+ get_commit_tree(c) +- c->maybe_tree = s ++ set_commit_tree(c, s) ...>} +// These excluded functions must access c->maybe_tree direcly. +// Note that if c->maybe_tree is written somewhere outside of these +// functions, then the recommended transformation will be bogus with +// get_commit_tree() on the LHS. @@ +identifier f !~ "^(get_commit_tree|get_commit_tree_in_graph_one|load_tree_for_commit|set_commit_tree)$"; expression c; -expression s; @@ -- get_commit_tree(c) = s -+ c->maybe_tree = s + f(...) {<... +- c->maybe_tree ++ get_commit_tree(c) + ...>} |