From: Junio C Hamano <gitster@pobox.com>
Date: Fri, 03 Jan 2014 18:26:43 GMT
Subject: Re: [PATCH] get_octopus_merge_bases(): cleanup redundant variable
Message-ID: <xmqqeh4oncpo.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1388760346-19515-1-git-send-email-einmalfel@gmail.com>

Vasily Makarov <einmalfel@gmail.com> writes:

> pptr is needless. Some related code got cleaned as well
>
> Signed-off-by: Vasily Makarov <einmalfel@gmail.com>
> ---
>  commit.c | 33 +++++++++++++++------------------
>  1 file changed, 15 insertions(+), 18 deletions(-)
>
> diff --git a/commit.c b/commit.c
> index de16a3c..4a7a192 100644
> --- a/commit.c
> +++ b/commit.c
> @@ -834,26 +834,23 @@ static struct commit_list *merge_bases_many(struct commit *one, int n, struct co
>  struct commit_list *get_octopus_merge_bases(struct commit_list *in)
>  {
>  	struct commit_list *i, *j, *k, *ret = NULL;
> -	struct commit_list **pptr = &ret;
>  
> -	for (i = in; i; i = i->next) {
> -		if (!ret)
> -			pptr = &commit_list_insert(i->item, pptr)->next;
> -		else {
> -			struct commit_list *new = NULL, *end = NULL;
> -
> -			for (j = ret; j; j = j->next) {
> -				struct commit_list *bases;
> -				bases = get_merge_bases(i->item, j->item, 1);
> -				if (!new)
> -					new = bases;
> -				else
> -					end->next = bases;
> -				for (k = bases; k; k = k->next)
> -					end = k;
> -			}
> -			ret = new;
> +	commit_list_insert(in->item, &ret);

I suspect that the original code would have behaved well (and I also
suspect that it was designed to) even if in==NULL upon entry, but
this version will crash here.  Nothing a simple

	if (!in)
        	return NULL;

upfront cannot fix, though.

And if we add these three lines back, the patch will become 18
insertions with 18 deletions but the result is very much more
readable than the original ;-).

> +
> +	for (i = in->next; i; i = i->next) {
> +		struct commit_list *new = NULL, *end = NULL;
> +
> +		for (j = ret; j; j = j->next) {
> +			struct commit_list *bases;
> +			bases = get_merge_bases(i->item, j->item, 1);
> +			if (!new)
> +				new = bases;
> +			else
> +				end->next = bases;
> +			for (k = bases; k; k = k->next)
> +				end = k;
>  		}
> +		ret = new;
>  	}
>  	return ret;
>  }