Re: [PATCH] get_octopus_merge_bases(): cleanup redundant variable
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 3, 2014, 18:26 UTC
- 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:
Show 35 quoted lines
> 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 ;-).
Show 18 quoted lines
> +
> + 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;
> }