Re: [PATCH 03/15] do not create "struct commit" with xcalloc

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> In both blame and merge-recursive, we sometimes create a
> "fake" commit struct for convenience (e.g., to represent the
> HEAD state as if we would commit it). By allocating
> ourselves rather than using alloc_commit_node, we do not
> properly set the "index" field of the commit. This can
> produce subtle bugs if we then use commit-slab on the
> resulting commit, as we will share the "0" index with
> another commit.

The change I see here is all good, but I wonder if it is too late to
start the real index from 1 and catch anything that has 0 index with
a BUG("do not hand-allocate a commit").

> We can fix this by using alloc_commit_node() to allocate.
> Note that we cannot free the result, as it is part of our
> commit allocator. However, both cases were already leaking
> the allocated commit anyway, so there's nothing to fix up.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  builtin/blame.c   | 2 +-
>  merge-recursive.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/blame.c b/builtin/blame.c
> index a52a279..d6056a5 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -2286,7 +2286,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,
>  	struct strbuf msg = STRBUF_INIT;
>  
>  	time(&now);
> -	commit = xcalloc(1, sizeof(*commit));
> +	commit = alloc_commit_node();
>  	commit->object.parsed = 1;
>  	commit->date = now;
>  	commit->object.type = OBJ_COMMIT;
> diff --git a/merge-recursive.c b/merge-recursive.c
> index cab16fa..2b37d42 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -40,7 +40,7 @@ static struct tree *shift_tree_object(struct tree *one, struct tree *two,
>  
>  static struct commit *make_virtual_commit(struct tree *tree, const char *comment)
>  {
> -	struct commit *commit = xcalloc(1, sizeof(struct commit));
> +	struct commit *commit = alloc_commit_node();
>  	struct merge_remote_desc *desc = xmalloc(sizeof(*desc));
>  
>  	desc->name = comment;