{"thread":{"id":"24466","subject":"[PATCH 0/7] find commit subject refactoring","startedAt":"2010-07-22T13:18:28Z","lastAt":"2010-07-23T09:37:59Z","messageCount":12,"participants":["Christian Couder","Peter Baumann","Junio C Hamano","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":7},"messages":[{"id":"145986","messageId":"20100722131141.2148.63850.chriscool@tuxfamily.org","threadId":"24466","inReplyTo":null,"subject":"[PATCH 0/7] find commit subject refactoring","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-07-22T13:18:28Z","receivedAt":"2010-07-22T13:18:28Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"I found 4 different places where there was custom code to find the\nsubject (sometimes called title) of a commit in the commit buffer.\nSo the purpose of this series is to refactor this by using a\ncommon function called find_commit_subject(), except for the first\npatch that is bug fix.\n\nChristian Couder (7):\n  revert: fix off by one read when searching the end of a commit\n    subject\n  revert: refactor code to find commit subject in find_commit_subject()\n  revert: rename subject related variables in get_message()\n  commit: move find_commit_subject() into commit.{h,c}\n  bisect: use find_commit_subject() instead of custom code\n  merge-recursive: use find_commit_subject() instead of custom code\n  blame: use find_commit_subject() instead of custom code\n\n bisect.c                     |   13 +++++--------\n builtin/blame.c              |   22 +++++++---------------\n builtin/revert.c             |   20 +++++---------------\n commit.c                     |   19 +++++++++++++++++++\n commit.h                     |    3 +++\n merge-recursive.c            |   14 ++++----------\n t/t3505-cherry-pick-empty.sh |   20 +++++++++++++++++++-\n 7 files changed, 62 insertions(+), 49 deletions(-)\n\n-- \n1.7.2.rc3.267.g400b3\n"},{"id":"145987","messageId":"20100722131836.2148.57468.chriscool@tuxfamily.org","threadId":"24466","inReplyTo":"20100722131141.2148.63850.chriscool@tuxfamily.org","subject":"[PATCH 1/7] revert: fix off by one read when searching the end of a commit subject","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-07-22T13:18:29Z","receivedAt":"2010-07-22T13:18:29Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"A test case is added but the problem can only be seen when running\nthe test case with --valgrind.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/revert.c             |    2 +-\n t/t3505-cherry-pick-empty.sh |   20 +++++++++++++++++++-\n 2 files changed, 20 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 8b9d829..3092233 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -131,7 +131,7 @@ static int get_message(const char *raw_message, struct commit_message *out)\n \t\tp++;\n \tif (*p) {\n \t\tp += 2;\n-\t\tfor (eol = p + 1; *eol && *eol != '\\n'; eol++)\n+\t\tfor (eol = p; *eol && *eol != '\\n'; eol++)\n \t\t\t; /* do nothing */\n \t} else\n \t\teol = p;\ndiff --git a/t/t3505-cherry-pick-empty.sh b/t/t3505-cherry-pick-empty.sh\nindex e51e505..c10b28c 100755\n--- a/t/t3505-cherry-pick-empty.sh\n+++ b/t/t3505-cherry-pick-empty.sh\n@@ -13,12 +13,30 @@ test_expect_success setup '\n \n \tgit checkout -b empty-branch &&\n \ttest_tick &&\n-\tgit commit --allow-empty -m \"empty\"\n+\tgit commit --allow-empty -m \"empty\" &&\n+\n+\techo third >> file1 &&\n+\tgit add file1 &&\n+\ttest_tick &&\n+\tgit commit --allow-empty-message -m \"\"\n \n '\n \n test_expect_success 'cherry-pick an empty commit' '\n \tgit checkout master && {\n+\t\tgit cherry-pick empty-branch^\n+\t\ttest \"$?\" = 1\n+\t}\n+'\n+\n+test_expect_success 'index lockfile was removed' '\n+\n+\ttest ! -f .git/index.lock\n+\n+'\n+\n+test_expect_success 'cherry-pick a commit with an empty message' '\n+\tgit checkout master && {\n \t\tgit cherry-pick empty-branch\n \t\ttest \"$?\" = 1\n \t}\n-- \n1.7.2.rc3.267.g400b3\n"},{"id":"145988","messageId":"20100722131836.2148.58435.chriscool@tuxfamily.org","threadId":"24466","inReplyTo":"20100722131141.2148.63850.chriscool@tuxfamily.org","subject":"[PATCH 2/7] revert: refactor code to find commit subject in find_commit_subject()","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-07-22T13:18:30Z","receivedAt":"2010-07-22T13:18:30Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/revert.c |   34 ++++++++++++++++++++++------------\n 1 files changed, 22 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 3092233..ed89bba 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -99,10 +99,30 @@ struct commit_message {\n \tconst char *message;\n };\n \n+/* Find beginning and length of commit subject. */\n+static int find_commit_subject(const char *commit_buffer, const char **subject)\n+{\n+\tconst char *eol;\n+\tconst char *p = commit_buffer;\n+\n+\twhile (*p && (*p != '\\n' || p[1] != '\\n'))\n+\t\tp++;\n+\tif (*p) {\n+\t\tp += 2;\n+\t\tfor (eol = p; *eol && *eol != '\\n'; eol++)\n+\t\t\t; /* do nothing */\n+\t} else\n+\t\teol = p;\n+\n+\t*subject = p;\n+\n+\treturn eol - p;\n+}\n+\n static int get_message(const char *raw_message, struct commit_message *out)\n {\n \tconst char *encoding;\n-\tconst char *p, *abbrev, *eol;\n+\tconst char *p, *abbrev;\n \tchar *q;\n \tint abbrev_len, oneline_len;\n \n@@ -125,17 +145,7 @@ static int get_message(const char *raw_message, struct commit_message *out)\n \tabbrev = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV);\n \tabbrev_len = strlen(abbrev);\n \n-\t/* Find beginning and end of commit subject. */\n-\tp = out->message;\n-\twhile (*p && (*p != '\\n' || p[1] != '\\n'))\n-\t\tp++;\n-\tif (*p) {\n-\t\tp += 2;\n-\t\tfor (eol = p; *eol && *eol != '\\n'; eol++)\n-\t\t\t; /* do nothing */\n-\t} else\n-\t\teol = p;\n-\toneline_len = eol - p;\n+\toneline_len = find_commit_subject(out->message, &p);\n \n \tout->parent_label = xmalloc(strlen(\"parent of \") + abbrev_len +\n \t\t\t      strlen(\"... \") + oneline_len + 1);\n-- \n1.7.2.rc3.267.g400b3\n"},{"id":"145989","messageId":"20100722131836.2148.57917.chriscool@tuxfamily.org","threadId":"24466","inReplyTo":"20100722131141.2148.63850.chriscool@tuxfamily.org","subject":"[PATCH 3/7] revert: rename subject related variables in get_message()","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-07-22T13:18:31Z","receivedAt":"2010-07-22T13:18:31Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/revert.c |   10 +++++-----\n 1 files changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex ed89bba..44149b5 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -122,9 +122,9 @@ static int find_commit_subject(const char *commit_buffer, const char **subject)\n static int get_message(const char *raw_message, struct commit_message *out)\n {\n \tconst char *encoding;\n-\tconst char *p, *abbrev;\n+\tconst char *abbrev, *subject;\n+\tint abbrev_len, subject_len;\n \tchar *q;\n-\tint abbrev_len, oneline_len;\n \n \tif (!raw_message)\n \t\treturn -1;\n@@ -145,17 +145,17 @@ static int get_message(const char *raw_message, struct commit_message *out)\n \tabbrev = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV);\n \tabbrev_len = strlen(abbrev);\n \n-\toneline_len = find_commit_subject(out->message, &p);\n+\tsubject_len = find_commit_subject(out->message, &subject);\n \n \tout->parent_label = xmalloc(strlen(\"parent of \") + abbrev_len +\n-\t\t\t      strlen(\"... \") + oneline_len + 1);\n+\t\t\t      strlen(\"... \") + subject_len + 1);\n \tq = out->parent_label;\n \tq = mempcpy(q, \"parent of \", strlen(\"parent of \"));\n \tout->label = q;\n \tq = mempcpy(q, abbrev, abbrev_len);\n \tq = mempcpy(q, \"... \", strlen(\"... \"));\n \tout->subject = q;\n-\tq = mempcpy(q, p, oneline_len);\n+\tq = mempcpy(q, subject, subject_len);\n \t*q = '\\0';\n \treturn 0;\n }\n-- \n1.7.2.rc3.267.g400b3\n"},{"id":"145990","messageId":"20100722131836.2148.80613.chriscool@tuxfamily.org","threadId":"24466","inReplyTo":"20100722131141.2148.63850.chriscool@tuxfamily.org","subject":"[PATCH 4/7] commit: move find_commit_subject() into commit.{h,c}","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-07-22T13:18:32Z","receivedAt":"2010-07-22T13:18:32Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/revert.c |   20 --------------------\n commit.c         |   19 +++++++++++++++++++\n commit.h         |    3 +++\n 3 files changed, 22 insertions(+), 20 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 44149b5..9215e66 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -99,26 +99,6 @@ struct commit_message {\n \tconst char *message;\n };\n \n-/* Find beginning and length of commit subject. */\n-static int find_commit_subject(const char *commit_buffer, const char **subject)\n-{\n-\tconst char *eol;\n-\tconst char *p = commit_buffer;\n-\n-\twhile (*p && (*p != '\\n' || p[1] != '\\n'))\n-\t\tp++;\n-\tif (*p) {\n-\t\tp += 2;\n-\t\tfor (eol = p; *eol && *eol != '\\n'; eol++)\n-\t\t\t; /* do nothing */\n-\t} else\n-\t\teol = p;\n-\n-\t*subject = p;\n-\n-\treturn eol - p;\n-}\n-\n static int get_message(const char *raw_message, struct commit_message *out)\n {\n \tconst char *encoding;\ndiff --git a/commit.c b/commit.c\nindex e9b0750..0094ec1 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -315,6 +315,25 @@ int parse_commit(struct commit *item)\n \treturn ret;\n }\n \n+int find_commit_subject(const char *commit_buffer, const char **subject)\n+{\n+\tconst char *eol;\n+\tconst char *p = commit_buffer;\n+\n+\twhile (*p && (*p != '\\n' || p[1] != '\\n'))\n+\t\tp++;\n+\tif (*p) {\n+\t\tp += 2;\n+\t\tfor (eol = p; *eol && *eol != '\\n'; eol++)\n+\t\t\t; /* do nothing */\n+\t} else\n+\t\teol = p;\n+\n+\t*subject = p;\n+\n+\treturn eol - p;\n+}\n+\n struct commit_list *commit_list_insert(struct commit *item, struct commit_list **list_p)\n {\n \tstruct commit_list *new_list = xmalloc(sizeof(struct commit_list));\ndiff --git a/commit.h b/commit.h\nindex eb2b8ac..9113bbe 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -41,6 +41,9 @@ int parse_commit_buffer(struct commit *item, void *buffer, unsigned long size);\n \n int parse_commit(struct commit *item);\n \n+/* Find beginning and length of commit subject. */\n+int find_commit_subject(const char *commit_buffer, const char **subject);\n+\n struct commit_list * commit_list_insert(struct commit *item, struct commit_list **list_p);\n unsigned commit_list_count(const struct commit_list *l);\n struct commit_list * insert_by_date(struct commit *item, struct commit_list **list);\n-- \n1.7.2.rc3.267.g400b3\n"},{"id":"145991","messageId":"20100722131836.2148.55405.chriscool@tuxfamily.org","threadId":"24466","inReplyTo":"20100722131141.2148.63850.chriscool@tuxfamily.org","subject":"[PATCH 5/7] bisect: use find_commit_subject() instead of custom code","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-07-22T13:18:33Z","receivedAt":"2010-07-22T13:18:33Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n bisect.c |   13 +++++--------\n 1 files changed, 5 insertions(+), 8 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex b556b11..060c042 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -141,7 +141,8 @@ static void show_list(const char *debug, int counted, int nr,\n \t\tenum object_type type;\n \t\tunsigned long size;\n \t\tchar *buf = read_sha1_file(commit->object.sha1, &type, &size);\n-\t\tchar *ep, *sp;\n+\t\tconst char *subject_start;\n+\t\tint subject_len;\n \n \t\tfprintf(stderr, \"%c%c%c \",\n \t\t\t(flags & TREESAME) ? ' ' : 'T',\n@@ -156,13 +157,9 @@ static void show_list(const char *debug, int counted, int nr,\n \t\t\tfprintf(stderr, \" %.*s\", 8,\n \t\t\t\tsha1_to_hex(pp->item->object.sha1));\n \n-\t\tsp = strstr(buf, \"\\n\\n\");\n-\t\tif (sp) {\n-\t\t\tsp += 2;\n-\t\t\tfor (ep = sp; *ep && *ep != '\\n'; ep++)\n-\t\t\t\t;\n-\t\t\tfprintf(stderr, \" %.*s\", (int)(ep - sp), sp);\n-\t\t}\n+\t\tsubject_len = find_commit_subject(buf, &subject_start);\n+\t\tif (subject_len)\n+\t\t\tfprintf(stderr, \" %.*s\", subject_len, subject_start);\n \t\tfprintf(stderr, \"\\n\");\n \t}\n }\n-- \n1.7.2.rc3.267.g400b3\n"},{"id":"145993","messageId":"20100722131836.2148.12472.chriscool@tuxfamily.org","threadId":"24466","inReplyTo":"20100722131141.2148.63850.chriscool@tuxfamily.org","subject":"[PATCH 6/7] merge-recursive: use find_commit_subject() instead of custom code","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-07-22T13:18:34Z","receivedAt":"2010-07-22T13:18:34Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n merge-recursive.c |   14 ++++----------\n 1 files changed, 4 insertions(+), 10 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 529d345..08f666a 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -136,16 +136,10 @@ static void output_commit_title(struct merge_options *o, struct commit *commit)\n \t\tif (parse_commit(commit) != 0)\n \t\t\tprintf(\"(bad commit)\\n\");\n \t\telse {\n-\t\t\tconst char *s;\n-\t\t\tint len;\n-\t\t\tfor (s = commit->buffer; *s; s++)\n-\t\t\t\tif (*s == '\\n' && s[1] == '\\n') {\n-\t\t\t\t\ts += 2;\n-\t\t\t\t\tbreak;\n-\t\t\t\t}\n-\t\t\tfor (len = 0; s[len] && '\\n' != s[len]; len++)\n-\t\t\t\t; /* do nothing */\n-\t\t\tprintf(\"%.*s\\n\", len, s);\n+\t\t\tconst char *title;\n+\t\t\tint len = find_commit_subject(commit->buffer, &title);\n+\t\t\tif (len)\n+\t\t\t\tprintf(\"%.*s\\n\", len, title);\n \t\t}\n \t}\n }\n-- \n1.7.2.rc3.267.g400b3\n"},{"id":"145992","messageId":"20100722131836.2148.2717.chriscool@tuxfamily.org","threadId":"24466","inReplyTo":"20100722131141.2148.63850.chriscool@tuxfamily.org","subject":"[PATCH 7/7] blame: use find_commit_subject() instead of custom code","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-07-22T13:18:35Z","receivedAt":"2010-07-22T13:18:35Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/blame.c |   22 +++++++---------------\n 1 files changed, 7 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 01e62fd..437b1a4 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -1407,7 +1407,8 @@ static void get_commit_info(struct commit *commit,\n \t\t\t    int detailed)\n {\n \tint len;\n-\tchar *tmp, *endp, *reencoded, *message;\n+\tconst char *subject;\n+\tchar *reencoded, *message;\n \tstatic char author_name[1024];\n \tstatic char author_mail[1024];\n \tstatic char committer_name[1024];\n@@ -1449,22 +1450,13 @@ static void get_commit_info(struct commit *commit,\n \t\t    &ret->committer_time, &ret->committer_tz);\n \n \tret->summary = summary_buf;\n-\ttmp = strstr(message, \"\\n\\n\");\n-\tif (!tmp) {\n-\terror_out:\n+\tlen = find_commit_subject(message, &subject);\n+\tif (len && len < sizeof(summary_buf)) {\n+\t\tmemcpy(summary_buf, subject, len);\n+\t\tsummary_buf[len] = 0;\n+\t} else {\n \t\tsprintf(summary_buf, \"(%s)\", sha1_to_hex(commit->object.sha1));\n-\t\tfree(reencoded);\n-\t\treturn;\n \t}\n-\ttmp += 2;\n-\tendp = strchr(tmp, '\\n');\n-\tif (!endp)\n-\t\tendp = tmp + strlen(tmp);\n-\tlen = endp - tmp;\n-\tif (len >= sizeof(summary_buf) || len == 0)\n-\t\tgoto error_out;\n-\tmemcpy(summary_buf, tmp, len);\n-\tsummary_buf[len] = 0;\n \tfree(reencoded);\n }\n \n-- \n1.7.2.rc3.267.g400b3\n"},{"id":"146000","messageId":"20100722165012.GA4938@m62s10.vlinux.de","threadId":"24466","inReplyTo":"20100722131836.2148.58435.chriscool@tuxfamily.org","subject":"Re: [PATCH 2/7] revert: refactor code to find commit subject in find_commit_subject()","fromName":"Peter Baumann","fromEmail":"waste.manager@gmx.de","sentAt":"2010-07-22T16:50:12Z","receivedAt":"2010-07-22T16:50:12Z","isPatch":true,"sender":{"key":"waste.manager@gmx.de","avatar":null},"body":"\nWouldn't it be better to merge this with  [PATCH 4/7], so we won't create\nfind_commit_subject in revert.c and then immediatly move it to commit.c?\n"},{"id":"146006","messageId":"7vaapjgyu1.fsf@alter.siamese.dyndns.org","threadId":"24466","inReplyTo":"20100722131141.2148.63850.chriscool@tuxfamily.org","subject":"Re: [PATCH 0/7] find commit subject refactoring","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-07-22T18:23:34Z","receivedAt":"2010-07-22T18:23:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nice ;-)\n"},{"id":"146031","messageId":"20100722230000.GD19745@burratino","threadId":"24466","inReplyTo":"20100722131836.2148.57468.chriscool@tuxfamily.org","subject":"Re: [PATCH 1/7] revert: fix off by one read when searching the end of a commit subject","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-22T23:00:00Z","receivedAt":"2010-07-22T23:00:00Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Christian Couder wrote:\n\n> +++ b/builtin/revert.c\n> @@ -131,7 +131,7 @@ static int get_message(const char *raw_message, struct commit_message *out)\n>  \t\tp++;\n>  \tif (*p) {\n>  \t\tp += 2;\n> -\t\tfor (eol = p + 1; *eol && *eol != '\\n'; eol++)\n> +\t\tfor (eol = p; *eol && *eol != '\\n'; eol++)\n>  \t\t\t; /* do nothing */\n>  \t} else\n>  \t\teol = p;\n\nGood catch.  For what it’s worth, this and the rest of the series is\n\nAcked-by: Jonathan Nieder <jrnieder@gmail.com>\n\nThanks.\n"},{"id":"146046","messageId":"201007231138.00360.chriscool@tuxfamily.org","threadId":"24466","inReplyTo":"20100722165012.GA4938@m62s10.vlinux.de","subject":"Re: [PATCH 2/7] revert: refactor code to find commit subject in find_commit_subject()","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-07-23T09:37:59Z","receivedAt":"2010-07-23T09:37:59Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Thursday 22 July 2010 18:50:12 Peter Baumann wrote:\n> Wouldn't it be better to merge this with  [PATCH 4/7], so we won't create\n> find_commit_subject in revert.c and then immediatly move it to commit.c?\n\nYeah, I could have merged those 2 patches, but except for the first one I \ndeveloped them in the order I sent them. So I didn't think much about merging \nsome as it felt natural to send them as is.\n\nBest regards,\nChristian.\n"}]}