Re: [PATCH 03/15] do not create "struct commit" with xcalloc
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jun 10, 2014, 21:35 UTC
- Message-ID
- <xmqqbnu08n2j.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <20140609181008.GC20315@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 8 quoted lines
> 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").
Show 37 quoted lines
> 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;