{"thread":{"id":"9863","subject":"[PATCH 1/3] rev-list --bisect: Move finding bisection into do_find_bisection.","startedAt":"2007-09-15T10:59:57Z","lastAt":"2007-09-17T03:19:19Z","messageCount":3,"participants":["Christian Couder","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"53104","messageId":"20070915125957.0899841b.chriscool@tuxfamily.org","threadId":"9863","inReplyTo":null,"subject":"[PATCH 1/3] rev-list --bisect: Move finding bisection into do_find_bisection.","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2007-09-15T10:59:57Z","receivedAt":"2007-09-15T10:59:57Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This factorises some code and make a big function smaller.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin-rev-list.c |   40 +++++++++++++++++++++++++---------------\n 1 files changed, 25 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin-rev-list.c b/builtin-rev-list.c\nindex ac551d5..f32c712 100644\n--- a/builtin-rev-list.c\n+++ b/builtin-rev-list.c\n@@ -255,6 +255,9 @@ static void show_list(const char *debug, int counted, int nr,\n }\n #endif /* DEBUG_BISECT */\n \n+static struct commit_list *do_find_bisection(struct commit_list *list,\n+\t\t\t\t\t     int nr, int *weights);\n+\n /*\n  * zero or positive weight is the number of interesting commits it can\n  * reach, including itself.  Especially, weight = 0 means it does not\n@@ -272,7 +275,7 @@ static void show_list(const char *debug, int counted, int nr,\n static struct commit_list *find_bisection(struct commit_list *list,\n \t\t\t\t\t  int *reaches, int *all)\n {\n-\tint n, nr, on_list, counted, distance;\n+\tint nr, on_list;\n \tstruct commit_list *p, *best, *next, *last;\n \tint *weights;\n \n@@ -301,6 +304,25 @@ static struct commit_list *find_bisection(struct commit_list *list,\n \n \t*all = nr;\n \tweights = xcalloc(on_list, sizeof(*weights));\n+\n+\t/* Do the real work of finding bisection commit. */\n+\tbest = do_find_bisection(list, nr, weights);\n+\n+\tif (best)\n+\t\tbest->next = NULL;\n+\n+\t*reaches = weight(best);\n+\tfree(weights);\n+\n+\treturn best;\n+}\n+\n+static struct commit_list *do_find_bisection(struct commit_list *list,\n+\t\t\t\t\t     int nr, int *weights)\n+{\n+\tint n, counted, distance;\n+\tstruct commit_list *p, *best;\n+\n \tcounted = 0;\n \n \tfor (n = 0, p = list; p; p = p->next) {\n@@ -357,12 +379,8 @@ static struct commit_list *find_bisection(struct commit_list *list,\n \t\tweight_set(p, distance);\n \n \t\t/* Does it happen to be at exactly half-way? */\n-\t\tif (halfway(p, distance, nr)) {\n-\t\t\tp->next = NULL;\n-\t\t\t*reaches = distance;\n-\t\t\tfree(weights);\n+\t\tif (halfway(p, distance, nr))\n \t\t\treturn p;\n-\t\t}\n \t\tcounted++;\n \t}\n \n@@ -400,12 +418,8 @@ static struct commit_list *find_bisection(struct commit_list *list,\n \n \t\t\t/* Does it happen to be at exactly half-way? */\n \t\t\tdistance = weight(p);\n-\t\t\tif (halfway(p, distance, nr)) {\n-\t\t\t\tp->next = NULL;\n-\t\t\t\t*reaches = distance;\n-\t\t\t\tfree(weights);\n+\t\t\tif (halfway(p, distance, nr))\n \t\t\t\treturn p;\n-\t\t\t}\n \t\t}\n \t}\n \n@@ -425,12 +439,8 @@ static struct commit_list *find_bisection(struct commit_list *list,\n \t\tif (distance > counted) {\n \t\t\tbest = p;\n \t\t\tcounted = distance;\n-\t\t\t*reaches = weight(p);\n \t\t}\n \t}\n-\tif (best)\n-\t\tbest->next = NULL;\n-\tfree(weights);\n \treturn best;\n }\n \n-- \n1.5.3.1.20.g2a16-dirty\n"},{"id":"53178","messageId":"7vk5qr575a.fsf@gitster.siamese.dyndns.org","threadId":"9863","inReplyTo":"20070915125957.0899841b.chriscool@tuxfamily.org","subject":"Re: [PATCH 1/3] rev-list --bisect: Move finding bisection into do_find_bisection.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-16T08:47:13Z","receivedAt":"2007-09-16T08:47:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <chriscool@tuxfamily.org> writes:\n\n> This factorises some code and make a big function smaller.\n\nI think the refactoring itself makes sense, especially where it\nsimplifies the clean-up of weight array in early-return\ncodepath.  But I have a couple of comments, though.\n\n> +static struct commit_list *do_find_bisection(struct commit_list *list,\n> +\t\t\t\t\t     int nr, int *weights);\n> +\n>  /*\n>   * zero or positive weight is the number of interesting commits it can\n>   * reach, including itself.  Especially, weight = 0 means it does not\n\nThe comment whose top part we can see here talks about the magic\nvalues -1 and -2 used while do_find_bisection() after the\nrefactoring does its work, and these magic values are never\nvisible to the calling function.  You should move the comment to\nthe top of do_find_bisection() as well.\n\nAlso this forward declaration is unwarranted.  A bottom-up\nsequence to define do_find_bisection() first, then to define its\nsole caller find_bisection() next is easier to read at least for\nme.\n\nThe latter comment also applies to your other patch.\n"},{"id":"53247","messageId":"200709170519.19504.chriscool@tuxfamily.org","threadId":"9863","inReplyTo":"7vk5qr575a.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 1/3] rev-list --bisect: Move finding bisection into do_find_bisection.","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2007-09-17T03:19:19Z","receivedAt":"2007-09-17T03:19:19Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Le dimanche 16 septembre 2007, Junio C Hamano a écrit :\n> > +static struct commit_list *do_find_bisection(struct commit_list *list,\n> > +\t\t\t\t\t     int nr, int *weights);\n> > +\n> >  /*\n> >   * zero or positive weight is the number of interesting commits it can\n> >   * reach, including itself.  Especially, weight = 0 means it does not\n>\n> The comment whose top part we can see here talks about the magic\n> values -1 and -2 used while do_find_bisection() after the\n> refactoring does its work, and these magic values are never\n> visible to the calling function.  You should move the comment to\n> the top of do_find_bisection() as well.\n>\n> Also this forward declaration is unwarranted.  A bottom-up\n> sequence to define do_find_bisection() first, then to define its\n> sole caller find_bisection() next is easier to read at least for\n> me.\n>\n> The latter comment also applies to your other patch.\n\nAll right, I will send new patchs with these changes.\n\nThanks,\nChristian.\n"}]}