On Wed, Apr 03, 2019 at 06:34:35PM +0700, Nguyễn Thái Ngọc Duy wrote:
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
Oh, look, an empty commit message ;)
> ---
> commit.c | 5 +++--
> commit.h | 3 ++-
> contrib/coccinelle/commit.cocci | 4 ++--
> 3 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/commit.c b/commit.c
> index a5333c7ac6..f0a5506f04 100644
> --- a/commit.c
> +++ b/commit.c
> @@ -340,7 +340,8 @@ void free_commit_buffer(struct parsed_object_pool *pool, struct commit *commit)
> }
> }
>
> -struct tree *get_commit_tree(const struct commit *commit)
> +struct tree *repo_get_commit_tree(struct repository *r,
> + const struct commit *commit)
A rename to accomodate the additional 'struct repository*'
parameter. Ok.
> {
> if (commit->maybe_tree || !commit->object.parsed)
> return commit->maybe_tree;
> @@ -348,7 +349,7 @@ struct tree *get_commit_tree(const struct commit *commit)
> if (commit->graph_pos == COMMIT_NOT_FROM_GRAPH)
> BUG("commit has NULL tree, but was not loaded from commit-graph");
>
> - return get_commit_tree_in_graph(the_repository, commit);
> + return get_commit_tree_in_graph(r, commit);
> }
>
> struct object_id *get_commit_tree_oid(const struct commit *commit)
> diff --git a/commit.h b/commit.h
> index 42728c2906..b576201be8 100644
> --- a/commit.h
> +++ b/commit.h
> @@ -143,7 +143,8 @@ void repo_unuse_commit_buffer(struct repository *r,
> */
> void free_commit_buffer(struct parsed_object_pool *pool, struct commit *);
>
> -struct tree *get_commit_tree(const struct commit *);
> +struct tree *repo_get_commit_tree(struct repository *, const struct commit *);
> +#define get_commit_tree(c) repo_get_commit_tree(the_repository, c)
But we still keep 'get_commit_tree' around as a macro wrapper
defaulting to 'the_repository', leaving the 30+ callsites intact.
Good.
Furthermore, the comment describing the 'maybe_tree' field in 'struct
commit's declaration still suggest get_commit_tree():
* If the commit is loaded from the commit-graph file, then this
* member may be NULL. Only access it through get_commit_tree()
* or get_commit_tree_oid().
> 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?
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.
> ...>}
>
> @@
> --
> 2.21.0.479.g47ac719cd3
>
