Re: [PATCH v2 10/32] commit.c: add repo_get_commit_tree()

From: Duy Nguyen <pclouds@gmail.com>
Date: Apr 5, 2019, 09:32 UTC
Message-ID: <CACsJy8Aa_Zcawoz07FBWjUjYE+RbMpxwrgim-tvQhMZW9+h2gQ@mail.gmail.com>
In-Reply-To: <20190404170427.GQ32732@szeder.dev>

On Fri, Apr 5, 2019 at 12:04 AM SZEDER Gábor <szeder.dev@gmail.com> wrote:

Show 21 quoted lines

> >  struct object_id *get_commit_tree_oid(const struct commit *);
> >
> >  /*
> > diff --git a/contrib/coccinelle/commit.cocci b/contrib/coccinelle/commit.cocci
> > index c49aa558f0..f5bc639981 100644
> > --- a/contrib/coccinelle/commit.cocci
> > +++ b/contrib/coccinelle/commit.cocci
> > @@ -12,12 +12,12 @@ expression c;
> >
> >  // 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 !~ "^(repo_get_commit_tree|get_commit_tree_in_graph_one|load_tree_for_commit)$";
> >  expression c;
> >  @@
> >    f(...) {<...
> >  - c->maybe_tree
> > -+ get_commit_tree(c)
> > ++ repo_get_commit_tree(the_repository, c)
>
> So, why this change?

Because get_commit_tree() now becomes a compat wrapper (yes I'll fill in the commit message ;) and should be avoided.

Show 5 quoted lines

> It would also require furher changes to 'commit.cocci', in particular
> to the last semantic patch, which is supposed to ensure that
> get_commit_tree() doesn't end up on the LHS of an assignment, but with
> this change Coccinelle does suggest transfomations with
> repo_get_commit_tree() on the LHS.

Oooh.. I see now. I actually updated that then dropped, thinking that a function call cannot be on LHS and that conversion is for already-long-gone code anyway. But yeah when you stack that conversion on top of this, it makes sense that we need double conversion to avoid build error.

Since I will have to update this patch anyway, I'll update the commit message on the first cocci patch too.

Show 7 quoted lines

>
> >    ...>}
> >
> >  @@
> > --
> > 2.21.0.479.g47ac719cd3
> >

-- 
Duy