All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org
Cc: Josh Steadmon <steadmon@google.com>
Subject: [RFH] disable bogus .maybe_tree suggestion by Coccinelle?
Date: Tue, 30 Jan 2024 10:08:46 -0800	[thread overview]
Message-ID: <xmqq1q9ybsnl.fsf@gitster.g> (raw)

Josh at $WORK noticed that "make coccicheck" shows tons of false
positives to tell us to avoid direct access to the .maybe_tree
member of a commit object, even for those that are meant to be
excluded from the rewrite rule, like this:

        diff -u -p a/commit-graph.c b/commit-graph.c
        --- a/commit-graph.c
        +++ b/commit-graph.c
        @@ -905,7 +905,7 @@ static void fill_commit_graph_info(struc

         static inline void set_commit_tree(struct commit *c, struct tree *t)
         {
        -	c->maybe_tree = t;
        +	repo_get_commit_tree(specify_the_right_repo_here, c) = t;
         }

         static int fill_commit_in_graph(struct repository *r,

It turns out that "make coccicheck" shows the same undesirable
suggestions, even in a checkout of 301b8c7f (commit.c: add
repo_get_commit_tree(), 2019-04-16) that added these rules.

If nobody has a better idea (which would obviously to "fix" it so
that we still are reminded to think twice before directly accessing
the .maybe_tree member without these false positives), I'd propose
to remove these rules.

The box I observed the sympotom had this

    $ spatch --version
    spatch version 1.1.1 compiled with OCaml version 4.14.1
    Flags passed to the configure script: --prefix=/usr --sysconfdir=/etc --libdir=/usr/lib --enable-ocaml --enable-python --with-python=python3 --enable-opt
    OCaml scripting support: yes
    Python scripting support: yes
    Syntax of regular expressions: Str


---
 contrib/coccinelle/commit.cocci | 23 -----------------------
 1 file changed, 23 deletions(-)

diff --git c/contrib/coccinelle/commit.cocci w/contrib/coccinelle/commit.cocci
index af6dd4c20c..397f01b9ff 100644
--- c/contrib/coccinelle/commit.cocci
+++ w/contrib/coccinelle/commit.cocci
@@ -10,29 +10,6 @@ expression c;
 - c->maybe_tree->object.oid.hash
 + get_commit_tree_oid(c)->hash
 
-@@
-identifier f !~ "^set_commit_tree$";
-expression c;
-expression s;
-@@
-  f(...) {<...
-- c->maybe_tree = s
-+ set_commit_tree(c, s)
-  ...>}
-
-// These excluded functions must access c->maybe_tree directly.
-// Note that if c->maybe_tree is written somewhere outside of these
-// functions, then the recommended transformation will be bogus with
-// repo_get_commit_tree() on the LHS.
-@@
-identifier f !~ "^(repo_get_commit_tree|get_commit_tree_in_graph_one|load_tree_for_commit|set_commit_tree)$";
-expression c;
-@@
-  f(...) {<...
-- c->maybe_tree
-+ repo_get_commit_tree(specify_the_right_repo_here, c)
-  ...>}
-
 @@
 struct commit *c;
 expression E;

                 reply	other threads:[~2024-01-30 18:08 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq1q9ybsnl.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=steadmon@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.