{"thread":{"id":"18565","subject":"[PATCH 07/10] rev-list: call new \"filter_skip\" function","startedAt":"2009-03-26T04:55:49Z","lastAt":"2009-04-02T04:23:40Z","messageCount":9,"participants":["Christian Couder","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":10},"messages":[{"id":"109487","messageId":"20090326055549.e1f244d9.chriscool@tuxfamily.org","threadId":"18565","inReplyTo":null,"subject":"[PATCH 07/10] rev-list: call new \"filter_skip\" function","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-03-26T04:55:49Z","receivedAt":"2009-03-26T04:55:49Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This patch implements a new \"filter_skip\" function in C in\n\"bisect.c\" that will later replace the existing implementation in\nshell in \"git-bisect.sh\".\n\nAn array is used to store the skipped commits. But the array is\nnot yet fed anything.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n bisect.c           |   65 ++++++++++++++++++++++++++++++++++++++++++++++++++++\n bisect.h           |    6 ++++-\n builtin-rev-list.c |   30 ++++++++++++++++++++----\n 3 files changed, 95 insertions(+), 6 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex 27def7d..39189f2 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -4,6 +4,11 @@\n #include \"revision.h\"\n #include \"bisect.h\"\n \n+\n+static unsigned char (*skipped_sha1)[20];\n+static int skipped_sha1_nr;\n+static int skipped_sha1_alloc;\n+\n /* bits #0-15 in revision.h */\n \n #define COUNTED\t\t(1u<<16)\n@@ -386,3 +391,63 @@ struct commit_list *find_bisection(struct commit_list *list,\n \treturn best;\n }\n \n+static int skipcmp(const void *a, const void *b)\n+{\n+\treturn hashcmp(a, b);\n+}\n+\n+static void prepare_skipped(void)\n+{\n+\tqsort(skipped_sha1, skipped_sha1_nr, sizeof(*skipped_sha1), skipcmp);\n+}\n+\n+static int lookup_skipped(unsigned char *sha1)\n+{\n+\tint lo, hi;\n+\tlo = 0;\n+\thi = skipped_sha1_nr;\n+\twhile (lo < hi) {\n+\t\tint mi = (lo + hi) / 2;\n+\t\tint cmp = hashcmp(sha1, skipped_sha1[mi]);\n+\t\tif (!cmp)\n+\t\t\treturn mi;\n+\t\tif (cmp < 0)\n+\t\t\thi = mi;\n+\t\telse\n+\t\t\tlo = mi + 1;\n+\t}\n+\treturn -lo - 1;\n+}\n+\n+struct commit_list *filter_skipped(struct commit_list *list,\n+\t\t\t\t   struct commit_list **tried,\n+\t\t\t\t   int show_all)\n+{\n+\tstruct commit_list *filtered = NULL, **f = &filtered;\n+\n+\t*tried = NULL;\n+\n+\tif (!skipped_sha1_nr)\n+\t\treturn list;\n+\n+\tprepare_skipped();\n+\n+\twhile (list) {\n+\t\tstruct commit_list *next = list->next;\n+\t\tlist->next = NULL;\n+\t\tif (0 <= lookup_skipped(list->item->object.sha1)) {\n+\t\t\t/* Move current to tried list */\n+\t\t\t*tried = list;\n+\t\t\ttried = &list->next;\n+\t\t} else {\n+\t\t\tif (!show_all)\n+\t\t\t\treturn list;\n+\t\t\t/* Move current to filtered list */\n+\t\t\t*f = list;\n+\t\t\tf = &list->next;\n+\t\t}\n+\t\tlist = next;\n+\t}\n+\n+\treturn filtered;\n+}\ndiff --git a/bisect.h b/bisect.h\nindex 31c99fe..2489630 100644\n--- a/bisect.h\n+++ b/bisect.h\n@@ -5,7 +5,11 @@ extern struct commit_list *find_bisection(struct commit_list *list,\n \t\t\t\t\t  int *reaches, int *all,\n \t\t\t\t\t  int find_all);\n \n+extern struct commit_list *filter_skipped(struct commit_list *list,\n+\t\t\t\t\t  struct commit_list **tried,\n+\t\t\t\t\t  int show_all);\n+\n extern int show_bisect_vars(struct rev_info *revs, int reaches, int all,\n-\t\t\t    int show_all);\n+\t\t\t    int show_all, int show_tried);\n \n #endif\ndiff --git a/builtin-rev-list.c b/builtin-rev-list.c\nindex cdb0f9d..925d643 100644\n--- a/builtin-rev-list.c\n+++ b/builtin-rev-list.c\n@@ -226,14 +226,28 @@ static int estimate_bisect_steps(int all)\n \treturn (e < 3 * x) ? n : n - 1;\n }\n \n-int show_bisect_vars(struct rev_info *revs, int reaches, int all, int show_all)\n+static void show_tried_revs(struct commit_list *tried)\n+{\n+\tprintf(\"bisect_tried='\");\n+\tfor (;tried; tried = tried->next) {\n+\t\tchar *format = tried->next ? \"%s|\" : \"%s\";\n+\t\tprintf(format, sha1_to_hex(tried->item->object.sha1));\n+\t}\n+\tprintf(\"'\\n\");\n+}\n+\n+int show_bisect_vars(struct rev_info *revs, int reaches, int all,\n+\t\t     int show_all, int show_tried)\n {\n \tint cnt;\n-\tchar hex[41];\n+\tchar hex[41] = \"\";\n+\tstruct commit_list *tried;\n \n-\tif (!revs->commits)\n+\tif (!revs->commits && !show_tried)\n \t\treturn 1;\n \n+\trevs->commits = filter_skipped(revs->commits, &tried, show_all);\n+\n \t/*\n \t * revs->commits can reach \"reaches\" commits among\n \t * \"all\" commits.  If it is good, then there are\n@@ -247,13 +261,16 @@ int show_bisect_vars(struct rev_info *revs, int reaches, int all, int show_all)\n \tif (cnt < reaches)\n \t\tcnt = reaches;\n \n-\tstrcpy(hex, sha1_to_hex(revs->commits->item->object.sha1));\n+\tif (revs->commits)\n+\t\tstrcpy(hex, sha1_to_hex(revs->commits->item->object.sha1));\n \n \tif (show_all) {\n \t\ttraverse_commit_list(revs, show_commit, show_object);\n \t\tprintf(\"------\\n\");\n \t}\n \n+\tif (show_tried)\n+\t\tshow_tried_revs(tried);\n \tprintf(\"bisect_rev=%s\\n\"\n \t       \"bisect_nr=%d\\n\"\n \t       \"bisect_good=%d\\n\"\n@@ -278,6 +295,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \tint bisect_list = 0;\n \tint bisect_show_vars = 0;\n \tint bisect_find_all = 0;\n+\tint bisect_show_all = 0;\n \tint quiet = 0;\n \n \tgit_config(git_default_config, NULL);\n@@ -305,6 +323,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \t\tif (!strcmp(arg, \"--bisect-all\")) {\n \t\t\tbisect_list = 1;\n \t\t\tbisect_find_all = 1;\n+\t\t\tbisect_show_all = 1;\n \t\t\trevs.show_decorations = 1;\n \t\t\tcontinue;\n \t\t}\n@@ -357,9 +376,10 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \n \t\trevs.commits = find_bisection(revs.commits, &reaches, &all,\n \t\t\t\t\t      bisect_find_all);\n+\n \t\tif (bisect_show_vars)\n \t\t\treturn show_bisect_vars(&revs, reaches, all,\n-\t\t\t\t\t\tbisect_find_all);\n+\t\t\t\t\t\tbisect_show_all, 0);\n \t}\n \n \ttraverse_commit_list(&revs,\n-- \n1.6.2.1.317.g3d804\n"},{"id":"109492","messageId":"7v8wmssrrs.fsf@gitster.siamese.dyndns.org","threadId":"18565","inReplyTo":"20090326055549.e1f244d9.chriscool@tuxfamily.org","subject":"Re: [PATCH 07/10] rev-list: call new \"filter_skip\" function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-26T06:49:11Z","receivedAt":"2009-03-26T06:49:11Z","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 patch implements a new \"filter_skip\" function in C in\n> \"bisect.c\" that will later replace the existing implementation in\n> shell in \"git-bisect.sh\".\n>\n> An array is used to store the skipped commits. But the array is\n> not yet fed anything.\n>\n> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n> ---\n>  bisect.c           |   65 ++++++++++++++++++++++++++++++++++++++++++++++++++++\n>  bisect.h           |    6 ++++-\n>  builtin-rev-list.c |   30 ++++++++++++++++++++----\n>  3 files changed, 95 insertions(+), 6 deletions(-)\n>\n> diff --git a/bisect.c b/bisect.c\n> index 27def7d..39189f2 100644\n> --- a/bisect.c\n> +++ b/bisect.c\n> @@ -4,6 +4,11 @@\n>  #include \"revision.h\"\n>  #include \"bisect.h\"\n>  \n> +\n> +static unsigned char (*skipped_sha1)[20];\n> +static int skipped_sha1_nr;\n> +static int skipped_sha1_alloc;\n> +\n>  /* bits #0-15 in revision.h */\n>  \n>  #define COUNTED\t\t(1u<<16)\n> @@ -386,3 +391,63 @@ struct commit_list *find_bisection(struct commit_list *list,\n>  \treturn best;\n>  }\n>  \n> +static int skipcmp(const void *a, const void *b)\n> +{\n> +\treturn hashcmp(a, b);\n> +}\n\nI've learned to suspect without reading a qsort() callback that does not\nderefence its arguments.  Is this doing the right thing?\n\n> +\n> +static void prepare_skipped(void)\n> +{\n> +\tqsort(skipped_sha1, skipped_sha1_nr, sizeof(*skipped_sha1), skipcmp);\n> +}\n> +\n> +static int lookup_skipped(unsigned char *sha1)\n> +{\n> +\tint lo, hi;\n> +\tlo = 0;\n> +\thi = skipped_sha1_nr;\n> +\twhile (lo < hi) {\n> +\t\tint mi = (lo + hi) / 2;\n> +\t\tint cmp = hashcmp(sha1, skipped_sha1[mi]);\n> +\t\tif (!cmp)\n> +\t\t\treturn mi;\n> +\t\tif (cmp < 0)\n> +\t\t\thi = mi;\n> +\t\telse\n> +\t\t\tlo = mi + 1;\n> +\t}\n> +\treturn -lo - 1;\n> +}\n"},{"id":"109807","messageId":"200903300526.12978.chriscool@tuxfamily.org","threadId":"18565","inReplyTo":"7v8wmssrrs.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 07/10] rev-list: call new \"filter_skip\" function","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-03-30T03:26:12Z","receivedAt":"2009-03-30T03:26:12Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Le jeudi 26 mars 2009, Junio C Hamano a écrit :\n> Christian Couder <chriscool@tuxfamily.org> writes:\n> > This patch implements a new \"filter_skip\" function in C in\n> > \"bisect.c\" that will later replace the existing implementation in\n> > shell in \"git-bisect.sh\".\n> >\n> > An array is used to store the skipped commits. But the array is\n> > not yet fed anything.\n> >\n> > Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n> > ---\n> >  bisect.c           |   65\n> > ++++++++++++++++++++++++++++++++++++++++++++++++++++ bisect.h          \n> > |    6 ++++-\n> >  builtin-rev-list.c |   30 ++++++++++++++++++++----\n> >  3 files changed, 95 insertions(+), 6 deletions(-)\n> >\n> > diff --git a/bisect.c b/bisect.c\n> > index 27def7d..39189f2 100644\n> > --- a/bisect.c\n> > +++ b/bisect.c\n> > @@ -4,6 +4,11 @@\n> >  #include \"revision.h\"\n> >  #include \"bisect.h\"\n> >\n> > +\n> > +static unsigned char (*skipped_sha1)[20];\n> > +static int skipped_sha1_nr;\n> > +static int skipped_sha1_alloc;\n> > +\n> >  /* bits #0-15 in revision.h */\n> >\n> >  #define COUNTED\t\t(1u<<16)\n> > @@ -386,3 +391,63 @@ struct commit_list *find_bisection(struct\n> > commit_list *list, return best;\n> >  }\n> >\n> > +static int skipcmp(const void *a, const void *b)\n> > +{\n> > +\treturn hashcmp(a, b);\n> > +}\n>\n> I've learned to suspect without reading a qsort() callback that does not\n> derefence its arguments.  Is this doing the right thing?\n\nI think so.\n\nHere is a gdb session that seems to show that it's ok:\n\n---------------------------------------------------\n...\n$ git bisect skip\nThere are only 'skip'ped commit left to test.\nThe first bad commit could be any of:\n3de952f2416b6084f557ec417709eac740c6818c\n7b7f204a749c3125d5224ed61ea2ae1187ad046f\n32a594a3fdac2d57cf6d02987e30eec68511498c\nWe cannot bisect more!\n$\n$ gdb git\nGNU gdb 6.6-debian\nCopyright (C) 2006 Free Software Foundation, Inc.\nGDB is free software, covered by the GNU General Public License, and you are\nwelcome to change it and/or distribute copies of it under certain \nconditions.\nType \"show copying\" to see the conditions.\nThere is absolutely no warranty for GDB.  Type \"show warranty\" for details.\nThis GDB was configured as \"x86_64-linux-gnu\"...\nUsing host libthread_db library \"/lib/libthread_db.so.1\".\n(gdb) b skipcmp\nBreakpoint 1 at 0x46edfd: file bisect.c, line 457.\n(gdb) run bisect--helper --next-vars\nStarting program: /home/christian/git/bin/git bisect--helper --next-vars\n[Thread debugging using libthread_db enabled]\n[New Thread 47665987240336 (LWP 9703)]\n[Switching to Thread 47665987240336 (LWP 9703)]\n\nBreakpoint 1, skipcmp (a=0x77a800, b=0x77a814) at bisect.c:457\n457             return hashcmp(a, b);\n(gdb) p skipped_sha1\n$1 = (unsigned char (*)[20]) 0x77a800\n(gdb) p a - b\n$2 = -20\n(gdb) p sha1_to_hex(a)\n$3 = 0x771256 \"3de952f2416b6084f557ec417709eac740c6818c\"\n(gdb) p sha1_to_hex(b)\n$4 = 0x7711c0 \"7b7f204a749c3125d5224ed61ea2ae1187ad046f\"\n(gdb)    \n---------------------------------------------------\n\nDo you think I should add a comment and/or perhaps cast a and b into \"const \nunsigned char *\"?\n\nThanks,\nChristian.\n"},{"id":"109849","messageId":"alpine.DEB.1.00.0903300951460.7534@intel-tinevez-2-302","threadId":"18565","inReplyTo":"200903300526.12978.chriscool@tuxfamily.org","subject":"Re: [PATCH 07/10] rev-list: call new \"filter_skip\" function","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-03-30T07:54:28Z","receivedAt":"2009-03-30T07:54:28Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 30 Mar 2009, Christian Couder wrote:\n\n> Le jeudi 26 mars 2009, Junio C Hamano a écrit :\n> > Christian Couder <chriscool@tuxfamily.org> writes:\n> > > This patch implements a new \"filter_skip\" function in C in\n> > > \"bisect.c\" that will later replace the existing implementation in\n> > > shell in \"git-bisect.sh\".\n> > >\n> > > An array is used to store the skipped commits. But the array is\n> > > not yet fed anything.\n> > >\n> > > Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n> > > ---\n> > >  bisect.c           |   65\n> > > ++++++++++++++++++++++++++++++++++++++++++++++++++++ bisect.h          \n> > > |    6 ++++-\n> > >  builtin-rev-list.c |   30 ++++++++++++++++++++----\n> > >  3 files changed, 95 insertions(+), 6 deletions(-)\n> > >\n> > > diff --git a/bisect.c b/bisect.c\n> > > index 27def7d..39189f2 100644\n> > > --- a/bisect.c\n> > > +++ b/bisect.c\n> > > @@ -4,6 +4,11 @@\n> > >  #include \"revision.h\"\n> > >  #include \"bisect.h\"\n> > >\n> > > +\n> > > +static unsigned char (*skipped_sha1)[20];\n> > > +static int skipped_sha1_nr;\n> > > +static int skipped_sha1_alloc;\n> > > +\n> > >  /* bits #0-15 in revision.h */\n> > >\n> > >  #define COUNTED\t\t(1u<<16)\n> > > @@ -386,3 +391,63 @@ struct commit_list *find_bisection(struct\n> > > commit_list *list, return best;\n> > >  }\n> > >\n> > > +static int skipcmp(const void *a, const void *b)\n> > > +{\n> > > +\treturn hashcmp(a, b);\n> > > +}\n> >\n> > I've learned to suspect without reading a qsort() callback that does not\n> > derefence its arguments.  Is this doing the right thing?\n> \n> I think so.\n\nI suspect something much worse: you are trying to build a list of sha1s of \ncommits that need to be skipped, later to look up every commit via \nbinary search.\n\nBut it has been proven a lot of times that using a hash set is superior to \nthat approach, and even better: we already have the framework in place in \nthe form of struct decorate.\n\nCiao,\nDscho \"who is too busy to review patches ATM\""},{"id":"109976","messageId":"200903310845.23674.chriscool@tuxfamily.org","threadId":"18565","inReplyTo":"alpine.DEB.1.00.0903300951460.7534@intel-tinevez-2-302","subject":"Re: [PATCH 07/10] rev-list: call new \"filter_skip\" function","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-03-31T06:45:23Z","receivedAt":"2009-03-31T06:45:23Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Hi,\n\nLe lundi 30 mars 2009, Johannes Schindelin a écrit :\n> Hi,\n>\n> On Mon, 30 Mar 2009, Christian Couder wrote:\n> > Le jeudi 26 mars 2009, Junio C Hamano a écrit :\n> > >\n> > > I've learned to suspect without reading a qsort() callback that does\n> > > not derefence its arguments.  Is this doing the right thing?\n> >\n> > I think so.\n>\n> I suspect something much worse: you are trying to build a list of sha1s\n> of commits that need to be skipped, later to look up every commit via\n> binary search.\n>\n> But it has been proven a lot of times that using a hash set is superior\n> to that approach, and even better: we already have the framework in place\n> in the form of struct decorate.\n\nI had a look, and \"struct decorate\" can be used to store objects, but I want \nto store only sha1s.\n\nRegards,\nChristian.\n"},{"id":"109985","messageId":"alpine.DEB.1.00.0903311117400.10279@pacific.mpi-cbg.de","threadId":"18565","inReplyTo":"200903310845.23674.chriscool@tuxfamily.org","subject":"Re: [PATCH 07/10] rev-list: call new \"filter_skip\" function","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-03-31T09:23:18Z","receivedAt":"2009-03-31T09:23:18Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 31 Mar 2009, Christian Couder wrote:\n\n> Le lundi 30 mars 2009, Johannes Schindelin a écrit :\n>\n> > On Mon, 30 Mar 2009, Christian Couder wrote:\n> > > Le jeudi 26 mars 2009, Junio C Hamano a écrit :\n> > > >\n> > > > I've learned to suspect without reading a qsort() callback that \n> > > > does not derefence its arguments.  Is this doing the right thing?\n> > >\n> > > I think so.\n> >\n> > I suspect something much worse: you are trying to build a list of \n> > sha1s of commits that need to be skipped, later to look up every \n> > commit via binary search.\n> >\n> > But it has been proven a lot of times that using a hash set is \n> > superior to that approach, and even better: we already have the \n> > framework in place in the form of struct decorate.\n> \n> I had a look, and \"struct decorate\" can be used to store objects, but I \n> want to store only sha1s.\n\nNo, you want to _look up_ sha1s.  And struct decorate is not about storing \nobjects, but to attach things to objects.  From decorate.h:\n\n\tstruct object_decoration {\n\t        const struct object *base;\n\t        void *decoration;\n\t};\n\nSo, if you just do\n\n\tstruct decoration all_the_sha1s;\n\n\t...\n\n\t\tadd_decoration(&all_the_sha1s, &commit->object, sha1);\n\nyou can look up the sha1 much more efficiently than with binary searches \nlike this:\n\n\t\tunsigned char *sha1 = lookup_decoration(&all_the_sha1s,\n\t\t\t&commit->object);\n\nCiao,\nDscho\n"},{"id":"110089","messageId":"200904010809.41110.chriscool@tuxfamily.org","threadId":"18565","inReplyTo":"alpine.DEB.1.00.0903311117400.10279@pacific.mpi-cbg.de","subject":"Re: [PATCH 07/10] rev-list: call new \"filter_skip\" function","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-04-01T06:09:40Z","receivedAt":"2009-04-01T06:09:40Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Le mardi 31 mars 2009, Johannes Schindelin a écrit :\n> Hi,\n>\n> On Tue, 31 Mar 2009, Christian Couder wrote:\n> > Le lundi 30 mars 2009, Johannes Schindelin a écrit :\n> > > On Mon, 30 Mar 2009, Christian Couder wrote:\n> > > > Le jeudi 26 mars 2009, Junio C Hamano a écrit :\n> > > > > I've learned to suspect without reading a qsort() callback that\n> > > > > does not derefence its arguments.  Is this doing the right thing?\n> > > >\n> > > > I think so.\n> > >\n> > > I suspect something much worse: you are trying to build a list of\n> > > sha1s of commits that need to be skipped, later to look up every\n> > > commit via binary search.\n> > >\n> > > But it has been proven a lot of times that using a hash set is\n> > > superior to that approach, and even better: we already have the\n> > > framework in place in the form of struct decorate.\n> >\n> > I had a look, and \"struct decorate\" can be used to store objects, but I\n> > want to store only sha1s.\n>\n> No, you want to _look up_ sha1s.  And struct decorate is not about\n> storing objects, but to attach things to objects. \n\nThe problem is that I don't have any object to attach things to when I read \nthe bisect skip refs. I just need to store the sha1 from the skip refs in \nsome sha1 container.\n\nHere is the related code:\n\nstatic int register_ref(const char *refname, const unsigned char *sha1,\n\t\t\tint flags, void *cb_data)\n{\n\tif (!strcmp(refname, \"bad\")) {\n\t\t...\n\t} else if (!prefixcmp(refname, \"good-\")) {\n\t\t...\n\t} else if (!prefixcmp(refname, \"skip-\")) {\n\t\tALLOC_GROW(skipped_sha1, skipped_sha1_nr + 1,\n\t\t\t   skipped_sha1_alloc);\n\t\thashcpy(skipped_sha1[skipped_sha1_nr++], sha1);\n\t}\n\n\treturn 0;\n}\n\nstatic int read_bisect_refs(void)\n{\n\treturn for_each_bisect_ref(register_ref, NULL);\n}\n\nBest regards,\nChristian.\n"},{"id":"110141","messageId":"alpine.DEB.1.00.0904011631300.13502@intel-tinevez-2-302","threadId":"18565","inReplyTo":"200904010809.41110.chriscool@tuxfamily.org","subject":"Re: [PATCH 07/10] rev-list: call new \"filter_skip\" function","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-04-01T14:36:44Z","receivedAt":"2009-04-01T14:36:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 1 Apr 2009, Christian Couder wrote:\n\n> Le mardi 31 mars 2009, Johannes Schindelin a écrit :\n> > Hi,\n> >\n> > On Tue, 31 Mar 2009, Christian Couder wrote:\n> > > Le lundi 30 mars 2009, Johannes Schindelin a écrit :\n> > > > On Mon, 30 Mar 2009, Christian Couder wrote:\n> > > > > Le jeudi 26 mars 2009, Junio C Hamano a écrit :\n> > > > > > I've learned to suspect without reading a qsort() callback that\n> > > > > > does not derefence its arguments.  Is this doing the right thing?\n> > > > >\n> > > > > I think so.\n> > > >\n> > > > I suspect something much worse: you are trying to build a list of\n> > > > sha1s of commits that need to be skipped, later to look up every\n> > > > commit via binary search.\n> > > >\n> > > > But it has been proven a lot of times that using a hash set is\n> > > > superior to that approach, and even better: we already have the\n> > > > framework in place in the form of struct decorate.\n> > >\n> > > I had a look, and \"struct decorate\" can be used to store objects, but I\n> > > want to store only sha1s.\n> >\n> > No, you want to _look up_ sha1s.  And struct decorate is not about\n> > storing objects, but to attach things to objects. \n> \n> The problem is that I don't have any object to attach things to when I \n> read the bisect skip refs. I just need to store the sha1 from the skip \n> refs in some sha1 container.\n\nI see, so you do not want to parse the commits just to register them as \nskipped.\n\nFair enough.\n\nBut I still think that a hashmap/set would be better suited.\n\nIn any case, it should be refactored into something usable in all of \nlibgit.a.  You are basically duplicating the grafts code in commit.c, \nsharing that shortcoming that your code would be static again, not \nencouraging reusage.\n\nCiao,\nDscho"},{"id":"110188","messageId":"200904020623.40908.chriscool@tuxfamily.org","threadId":"18565","inReplyTo":"alpine.DEB.1.00.0904011631300.13502@intel-tinevez-2-302","subject":"Re: [PATCH 07/10] rev-list: call new \"filter_skip\" function","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-04-02T04:23:40Z","receivedAt":"2009-04-02T04:23:40Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Hi,\n\nLe mercredi 1 avril 2009, Johannes Schindelin a écrit :\n> Hi,\n>\n> On Wed, 1 Apr 2009, Christian Couder wrote:\n> > Le mardi 31 mars 2009, Johannes Schindelin a écrit :\n> > > Hi,\n> > >\n> > > On Tue, 31 Mar 2009, Christian Couder wrote:\n> > > > Le lundi 30 mars 2009, Johannes Schindelin a écrit :\n> > > No, you want to _look up_ sha1s.  And struct decorate is not about\n> > > storing objects, but to attach things to objects.\n> >\n> > The problem is that I don't have any object to attach things to when I\n> > read the bisect skip refs. I just need to store the sha1 from the skip\n> > refs in some sha1 container.\n>\n> I see, so you do not want to parse the commits just to register them as\n> skipped.\n>\n> Fair enough.\n>\n> But I still think that a hashmap/set would be better suited.\n>\n> In any case, it should be refactored into something usable in all of\n> libgit.a.  You are basically duplicating the grafts code in commit.c,\n> sharing that shortcoming that your code would be static again, not\n> encouraging reusage.\n\nI agree that binary search functions and related code should be refactored. \nThat's why I added the \"Refactor binary search functions\" task to the \nJanitor wiki page (http://git.or.cz/gitwiki/Janitor) a few weeks ago.\n\nI will have a look at that, and perhaps, after that, I will try to make \nthings less static, but I think these are some old problems in the code \nbase, so new developments should not be hostage of them.\n\nBest reagrds,\nChristian.\n"}]}