Jeff King <p...@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 <p...@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;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to