{"thread":{"id":"35601","subject":"[PATCH] get_octopus_merge_bases(): cleanup redundant variable","startedAt":"2014-01-03T14:45:46Z","lastAt":"2014-01-03T19:01:15Z","messageCount":3,"participants":["Vasily Makarov","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"232615","messageId":"1388760346-19515-1-git-send-email-einmalfel@gmail.com","threadId":"35601","inReplyTo":null,"subject":"[PATCH] get_octopus_merge_bases(): cleanup redundant variable","fromName":"Vasily Makarov","fromEmail":"einmalfel@gmail.com","sentAt":"2014-01-03T14:45:46Z","receivedAt":"2014-01-03T14:45:46Z","isPatch":true,"sender":{"key":"einmalfel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2640813?v=4"},"body":"pptr is needless. Some related code got cleaned as well\n\nSigned-off-by: Vasily Makarov <einmalfel@gmail.com>\n---\n commit.c | 33 +++++++++++++++------------------\n 1 file changed, 15 insertions(+), 18 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex de16a3c..4a7a192 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -834,26 +834,23 @@ static struct commit_list *merge_bases_many(struct commit *one, int n, struct co\n struct commit_list *get_octopus_merge_bases(struct commit_list *in)\n {\n \tstruct commit_list *i, *j, *k, *ret = NULL;\n-\tstruct commit_list **pptr = &ret;\n \n-\tfor (i = in; i; i = i->next) {\n-\t\tif (!ret)\n-\t\t\tpptr = &commit_list_insert(i->item, pptr)->next;\n-\t\telse {\n-\t\t\tstruct commit_list *new = NULL, *end = NULL;\n-\n-\t\t\tfor (j = ret; j; j = j->next) {\n-\t\t\t\tstruct commit_list *bases;\n-\t\t\t\tbases = get_merge_bases(i->item, j->item, 1);\n-\t\t\t\tif (!new)\n-\t\t\t\t\tnew = bases;\n-\t\t\t\telse\n-\t\t\t\t\tend->next = bases;\n-\t\t\t\tfor (k = bases; k; k = k->next)\n-\t\t\t\t\tend = k;\n-\t\t\t}\n-\t\t\tret = new;\n+\tcommit_list_insert(in->item, &ret);\n+\n+\tfor (i = in->next; i; i = i->next) {\n+\t\tstruct commit_list *new = NULL, *end = NULL;\n+\n+\t\tfor (j = ret; j; j = j->next) {\n+\t\t\tstruct commit_list *bases;\n+\t\t\tbases = get_merge_bases(i->item, j->item, 1);\n+\t\t\tif (!new)\n+\t\t\t\tnew = bases;\n+\t\t\telse\n+\t\t\t\tend->next = bases;\n+\t\t\tfor (k = bases; k; k = k->next)\n+\t\t\t\tend = k;\n \t\t}\n+\t\tret = new;\n \t}\n \treturn ret;\n }\n-- \n1.8.3.2\n"},{"id":"232625","messageId":"xmqqeh4oncpo.fsf@gitster.dls.corp.google.com","threadId":"35601","inReplyTo":"1388760346-19515-1-git-send-email-einmalfel@gmail.com","subject":"Re: [PATCH] get_octopus_merge_bases(): cleanup redundant variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-01-03T18:26:43Z","receivedAt":"2014-01-03T18:26:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Vasily Makarov <einmalfel@gmail.com> writes:\n\n> pptr is needless. Some related code got cleaned as well\n>\n> Signed-off-by: Vasily Makarov <einmalfel@gmail.com>\n> ---\n>  commit.c | 33 +++++++++++++++------------------\n>  1 file changed, 15 insertions(+), 18 deletions(-)\n>\n> diff --git a/commit.c b/commit.c\n> index de16a3c..4a7a192 100644\n> --- a/commit.c\n> +++ b/commit.c\n> @@ -834,26 +834,23 @@ static struct commit_list *merge_bases_many(struct commit *one, int n, struct co\n>  struct commit_list *get_octopus_merge_bases(struct commit_list *in)\n>  {\n>  \tstruct commit_list *i, *j, *k, *ret = NULL;\n> -\tstruct commit_list **pptr = &ret;\n>  \n> -\tfor (i = in; i; i = i->next) {\n> -\t\tif (!ret)\n> -\t\t\tpptr = &commit_list_insert(i->item, pptr)->next;\n> -\t\telse {\n> -\t\t\tstruct commit_list *new = NULL, *end = NULL;\n> -\n> -\t\t\tfor (j = ret; j; j = j->next) {\n> -\t\t\t\tstruct commit_list *bases;\n> -\t\t\t\tbases = get_merge_bases(i->item, j->item, 1);\n> -\t\t\t\tif (!new)\n> -\t\t\t\t\tnew = bases;\n> -\t\t\t\telse\n> -\t\t\t\t\tend->next = bases;\n> -\t\t\t\tfor (k = bases; k; k = k->next)\n> -\t\t\t\t\tend = k;\n> -\t\t\t}\n> -\t\t\tret = new;\n> +\tcommit_list_insert(in->item, &ret);\n\nI suspect that the original code would have behaved well (and I also\nsuspect that it was designed to) even if in==NULL upon entry, but\nthis version will crash here.  Nothing a simple\n\n\tif (!in)\n        \treturn NULL;\n\nupfront cannot fix, though.\n\nAnd if we add these three lines back, the patch will become 18\ninsertions with 18 deletions but the result is very much more\nreadable than the original ;-).\n\n> +\n> +\tfor (i = in->next; i; i = i->next) {\n> +\t\tstruct commit_list *new = NULL, *end = NULL;\n> +\n> +\t\tfor (j = ret; j; j = j->next) {\n> +\t\t\tstruct commit_list *bases;\n> +\t\t\tbases = get_merge_bases(i->item, j->item, 1);\n> +\t\t\tif (!new)\n> +\t\t\t\tnew = bases;\n> +\t\t\telse\n> +\t\t\t\tend->next = bases;\n> +\t\t\tfor (k = bases; k; k = k->next)\n> +\t\t\t\tend = k;\n>  \t\t}\n> +\t\tret = new;\n>  \t}\n>  \treturn ret;\n>  }\n"},{"id":"232628","messageId":"1388775675-12228-1-git-send-email-einmalfel@gmail.com","threadId":"35601","inReplyTo":"xmqqeh4oncpo.fsf@gitster.dls.corp.google.com","subject":"[PATCH] get_octopus_merge_bases(): cleanup redundant variable","fromName":"Vasily Makarov","fromEmail":"einmalfel@gmail.com","sentAt":"2014-01-03T19:01:15Z","receivedAt":"2014-01-03T19:01:15Z","isPatch":true,"sender":{"key":"einmalfel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2640813?v=4"},"body":"pptr is needless. Some related code got cleaned as well\n\nSigned-off-by: Vasily Makarov <einmalfel@gmail.com>\n---\n commit.c | 36 ++++++++++++++++++------------------\n 1 file changed, 18 insertions(+), 18 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex de16a3c..8e48c49 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -834,26 +834,26 @@ static struct commit_list *merge_bases_many(struct commit *one, int n, struct co\n struct commit_list *get_octopus_merge_bases(struct commit_list *in)\n {\n \tstruct commit_list *i, *j, *k, *ret = NULL;\n-\tstruct commit_list **pptr = &ret;\n \n-\tfor (i = in; i; i = i->next) {\n-\t\tif (!ret)\n-\t\t\tpptr = &commit_list_insert(i->item, pptr)->next;\n-\t\telse {\n-\t\t\tstruct commit_list *new = NULL, *end = NULL;\n-\n-\t\t\tfor (j = ret; j; j = j->next) {\n-\t\t\t\tstruct commit_list *bases;\n-\t\t\t\tbases = get_merge_bases(i->item, j->item, 1);\n-\t\t\t\tif (!new)\n-\t\t\t\t\tnew = bases;\n-\t\t\t\telse\n-\t\t\t\t\tend->next = bases;\n-\t\t\t\tfor (k = bases; k; k = k->next)\n-\t\t\t\t\tend = k;\n-\t\t\t}\n-\t\t\tret = new;\n+\tif (!in)\n+\t\treturn NULL;\n+\n+\tcommit_list_insert(in->item, &ret);\n+\n+\tfor (i = in->next; i; i = i->next) {\n+\t\tstruct commit_list *new = NULL, *end = NULL;\n+\n+\t\tfor (j = ret; j; j = j->next) {\n+\t\t\tstruct commit_list *bases;\n+\t\t\tbases = get_merge_bases(i->item, j->item, 1);\n+\t\t\tif (!new)\n+\t\t\t\tnew = bases;\n+\t\t\telse\n+\t\t\t\tend->next = bases;\n+\t\t\tfor (k = bases; k; k = k->next)\n+\t\t\t\tend = k;\n \t\t}\n+\t\tret = new;\n \t}\n \treturn ret;\n }\n-- \n1.8.3.2\n"}]}