Re: [PATCH] Introduce get_octopus_merge_bases() in commit.c

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jun 9, 2008, 14:02 UTC
Message-ID: <alpine.DEB.1.00.0806091458190.1783@racer>
In-Reply-To: <1212874720-22324-1-git-send-email-vmiklos@frugalware.org>

Hi,

On Sat, 7 Jun 2008, Miklos Vajna wrote:

Show 13 quoted lines

> diff --git a/commit.c b/commit.c
> index 1dba717..685a144 100644
> --- a/commit.c
> +++ b/commit.c
> @@ -618,6 +618,23 @@ static struct commit_list *merge_bases(struct commit *one, struct commit *two)
>  	return merge_bases_many(one, 1, &two);
>  }
>  
> +struct commit_list *get_octopus_merge_bases(struct commit_list *in, int cleanup)
> +{
> +	struct commit *head = in->item;
> +	int n = 0;
> +	struct commit **commits = xmalloc(n*sizeof(struct commit *));

Here, n will be 0 and therefore commits will be xmalloc(0), right?

> +	struct commit_list *ret, *i;
> +
> +	for(i = in; i; i = i->next, n++)
> +		commits[n] = i->item;

And here, commits will never be realloc()ed.

> +	ret = merge_bases_many(head, n, commits);

If merge_bases_many took a commit_list (yes, as I suggested to Junio), this transformation would not be necessary.

IIRC nothing in merge_bases_many() needed a commit array.

Oh, and whose responsibility is it to free "in"? Caller or callee? (Because it is a non-const parameter, I would have expected the callee, but I think it makes more sense if the caller can do whatever she wants with the heads after calling octopus_merge_bases()).

Ciao, Dscho