{"thread":{"id":"60817","subject":"[RFH] disable bogus .maybe_tree suggestion by Coccinelle?","startedAt":"2024-01-30T18:08:57Z","lastAt":"2024-01-30T18:08:57Z","messageCount":1,"participants":["Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"487604","messageId":"xmqq1q9ybsnl.fsf@gitster.g","threadId":"60817","inReplyTo":null,"subject":"[RFH] disable bogus .maybe_tree suggestion by Coccinelle?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-01-30T18:08:46Z","receivedAt":"2024-01-30T18:08:57Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Josh at $WORK noticed that \"make coccicheck\" shows tons of false\npositives to tell us to avoid direct access to the .maybe_tree\nmember of a commit object, even for those that are meant to be\nexcluded from the rewrite rule, like this:\n\n        diff -u -p a/commit-graph.c b/commit-graph.c\n        --- a/commit-graph.c\n        +++ b/commit-graph.c\n        @@ -905,7 +905,7 @@ static void fill_commit_graph_info(struc\n\n         static inline void set_commit_tree(struct commit *c, struct tree *t)\n         {\n        -\tc->maybe_tree = t;\n        +\trepo_get_commit_tree(specify_the_right_repo_here, c) = t;\n         }\n\n         static int fill_commit_in_graph(struct repository *r,\n\nIt turns out that \"make coccicheck\" shows the same undesirable\nsuggestions, even in a checkout of 301b8c7f (commit.c: add\nrepo_get_commit_tree(), 2019-04-16) that added these rules.\n\nIf nobody has a better idea (which would obviously to \"fix\" it so\nthat we still are reminded to think twice before directly accessing\nthe .maybe_tree member without these false positives), I'd propose\nto remove these rules.\n\nThe box I observed the sympotom had this\n\n    $ spatch --version\n    spatch version 1.1.1 compiled with OCaml version 4.14.1\n    Flags passed to the configure script: --prefix=/usr --sysconfdir=/etc --libdir=/usr/lib --enable-ocaml --enable-python --with-python=python3 --enable-opt\n    OCaml scripting support: yes\n    Python scripting support: yes\n    Syntax of regular expressions: Str\n\n\n---\n contrib/coccinelle/commit.cocci | 23 -----------------------\n 1 file changed, 23 deletions(-)\n\ndiff --git c/contrib/coccinelle/commit.cocci w/contrib/coccinelle/commit.cocci\nindex af6dd4c20c..397f01b9ff 100644\n--- c/contrib/coccinelle/commit.cocci\n+++ w/contrib/coccinelle/commit.cocci\n@@ -10,29 +10,6 @@ expression c;\n - c->maybe_tree->object.oid.hash\n + get_commit_tree_oid(c)->hash\n \n-@@\n-identifier f !~ \"^set_commit_tree$\";\n-expression c;\n-expression s;\n-@@\n-  f(...) {<...\n-- c->maybe_tree = s\n-+ set_commit_tree(c, s)\n-  ...>}\n-\n-// These excluded functions must access c->maybe_tree directly.\n-// Note that if c->maybe_tree is written somewhere outside of these\n-// functions, then the recommended transformation will be bogus with\n-// repo_get_commit_tree() on the LHS.\n-@@\n-identifier f !~ \"^(repo_get_commit_tree|get_commit_tree_in_graph_one|load_tree_for_commit|set_commit_tree)$\";\n-expression c;\n-@@\n-  f(...) {<...\n-- c->maybe_tree\n-+ repo_get_commit_tree(specify_the_right_repo_here, c)\n-  ...>}\n-\n @@\n struct commit *c;\n expression E;\n"}]}