{"thread":{"id":"35957","subject":"[PATCH] commit.c: use the generic \"sha1_pos\" function for lookup sha1","startedAt":"2014-02-26T10:07:47Z","lastAt":"2014-02-26T18:49:05Z","messageCount":4,"participants":["Dmitry S. Dolzhenko","Jeff King","Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"235358","messageId":"530DBCF3.9060801@yandex.ru","threadId":"35957","inReplyTo":null,"subject":"[PATCH] commit.c: use the generic \"sha1_pos\" function for lookup sha1","fromName":"Dmitry S. Dolzhenko","fromEmail":"dmitrys.dolzhenko@yandex.ru","sentAt":"2014-02-26T10:07:47Z","receivedAt":"2014-02-26T10:07:47Z","isPatch":true,"sender":{"key":"dmitrys.dolzhenko@yandex.ru","avatar":null},"body":"Refactor binary search in \"commit_graft_pos\" function: use\ngeneric \"sha1_pos\" function.\n\nSigned-off-by: Dmitry S. Dolzhenko \n---\n commit.c | 24 +++++++++---------------\n 1 file changed, 9 insertions(+), 15 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex 6bf4fe0..8edaeb7 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -10,6 +10,7 @@\n #include \"mergesort.h\"\n #include \"commit-slab.h\"\n #include \"prio-queue.h\"\n+#include \"sha1-lookup.h\"\n static struct commit_extra_header *read_commit_extra_header_lines(const char *buf, size_t len, const char **);\n @@ -114,23 +115,16 @@ static unsigned long parse_commit_date(const char *buf, const char *tail)\n static struct commit_graft **commit_graft;\n static int commit_graft_alloc, commit_graft_nr;\n +static const unsigned char *commit_graft_sha1_access(size_t index, void *table)\n+{\n+\tstruct commit_graft **commit_graft_table = table;\n+\treturn commit_graft_table[index]->sha1;\n+}\n+\n static int commit_graft_pos(const unsigned char *sha1)\n {\n-\tint lo, hi;\n-\tlo = 0;\n-\thi = commit_graft_nr;\n-\twhile (lo < hi) {\n-\t\tint mi = (lo + hi) / 2;\n-\t\tstruct commit_graft *graft = commit_graft[mi];\n-\t\tint cmp = hashcmp(sha1, graft->sha1);\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+\treturn sha1_pos(sha1, commit_graft, commit_graft_nr,\n+\t\t\t commit_graft_sha1_access);\n }\n int register_commit_graft(struct commit_graft *graft, int ignore_dups)\n-- \n1.8.5.3\n\n\n\n"},{"id":"235363","messageId":"20140226104150.GD25711@sigill.intra.peff.net","threadId":"35957","inReplyTo":"530DBCF3.9060801@yandex.ru","subject":"Re: [PATCH] commit.c: use the generic \"sha1_pos\" function for lookup sha1","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-02-26T10:41:51Z","receivedAt":"2014-02-26T10:41:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 26, 2014 at 02:07:47PM +0400, Dmitry S. Dolzhenko wrote:\n\n> Refactor binary search in \"commit_graft_pos\" function: use\n> generic \"sha1_pos\" function.\n\nSounds sensible.\n\nA few administrative points for your patch:\n\n - we usually try to send patches inline, rather than as attachments.\n It almost looks like your mailer or a server on the path converted\n your mail to a multipart/mixed and stuck a useless empty attachment\n on the end.\n\n - Your patch did not apply for me, nor to the blobs mentioned in its\n header. Did you modify it after it was generated? I think this part\n in particular looks suspicious:\n\n\n> diff --git a/commit.c b/commit.c\n> index 6bf4fe0..8edaeb7 100644\n> --- a/commit.c\n> +++ b/commit.c\n> @@ -10,6 +10,7 @@\n> #include \"mergesort.h\"\n> #include \"commit-slab.h\"\n> #include \"prio-queue.h\"\n> +#include \"sha1-lookup.h\"\n> static struct commit_extra_header *read_commit_extra_header_lines(const char *buf, size_t len, const char **);\n\nThere are 3 context lines above, but only one below?\n\n> @@ -114,23 +115,16 @@ static unsigned long parse_commit_date(const char *buf, const char *tail)\n> static struct commit_graft **commit_graft;\n> static int commit_graft_alloc, commit_graft_nr;\n> +static const unsigned char *commit_graft_sha1_access(size_t index, void *table)\n> +{\n> +\tstruct commit_graft **commit_graft_table = table;\n> +\treturn commit_graft_table[index]->sha1;\n> +}\n> +\n\nAnd here we have only two context lines, and the first addition line is\nindented (making it look like a context line).\n\n> static int commit_graft_pos(const unsigned char *sha1)\n> {\n> -\tint lo, hi;\n> -\tlo = 0;\n> -\thi = commit_graft_nr;\n> -\twhile (lo < hi) {\n> -\t\tint mi = (lo + hi) / 2;\n> -\t\tstruct commit_graft *graft = commit_graft[mi];\n> -\t\tint cmp = hashcmp(sha1, graft->sha1);\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> +\treturn sha1_pos(sha1, commit_graft, commit_graft_nr,\n> +\t\t\t commit_graft_sha1_access);\n\nThe patch itself looks correct.\n\n-Peff\n"},{"id":"235366","messageId":"CACsJy8BQKHuvLpCQMuVK79owdoTbHkS0k3yNJ_9fyvDiJooJgg@mail.gmail.com","threadId":"35957","inReplyTo":"530DBCF3.9060801@yandex.ru","subject":"Re: [PATCH] commit.c: use the generic \"sha1_pos\" function for lookup sha1","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-02-26T10:53:58Z","receivedAt":"2014-02-26T10:53:58Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Feb 26, 2014 at 5:07 PM, Dmitry S. Dolzhenko\n wrote:\n> Refactor binary search in \"commit_graft_pos\" function: use\n> generic \"sha1_pos\" function.\n\nFor fun, try to break your changes deliberately then run \"make test\"\nand see if the failed tests can lead you back to this function. I hear\nsome people gather in irc. I think they could help you faster when you\nneed to track this code. Else get support from this mailing list. If\nthe test suite does not detect it at all, you can add new tests to\ncover it.\n-- \nDuy\n"},{"id":"235399","messageId":"530E3721.6010502@yandex.ru","threadId":"35957","inReplyTo":"20140226104150.GD25711@sigill.intra.peff.net","subject":"Re: [PATCH] commit.c: use the generic \"sha1_pos\" function for lookup sha1","fromName":"Dmitry S. Dolzhenko","fromEmail":"dmitrys.dolzhenko@yandex.ru","sentAt":"2014-02-26T18:49:05Z","receivedAt":"2014-02-26T18:49:05Z","isPatch":true,"sender":{"key":"dmitrys.dolzhenko@yandex.ru","avatar":null},"body":"\nThank you for your remarks. I'll try to fix my patch and send it again.\n"}]}