From 2c048a3038fd4d94e150fbc1dfd2242325ca7db6 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Thu, 22 Jul 2010 15:18:29 +0200 Subject: [PATCH 1/6] revert: fix off by one read when searching the end of a commit subject A test case is added but the problem can only be seen when running the test case with --valgrind. Signed-off-by: Christian Couder Acked-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- builtin/revert.c | 2 +- t/t3505-cherry-pick-empty.sh | 20 +++++++++++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/builtin/revert.c b/builtin/revert.c index 7d68ef714e..c170715ba8 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -127,7 +127,7 @@ static int get_message(const char *raw_message, struct commit_message *out) p++; if (*p) { p += 2; - for (eol = p + 1; *eol && *eol != '\n'; eol++) + for (eol = p; *eol && *eol != '\n'; eol++) ; /* do nothing */ } else eol = p; diff --git a/t/t3505-cherry-pick-empty.sh b/t/t3505-cherry-pick-empty.sh index e51e505a9f..c10b28cf57 100755 --- a/t/t3505-cherry-pick-empty.sh +++ b/t/t3505-cherry-pick-empty.sh @@ -13,11 +13,29 @@ test_expect_success setup ' git checkout -b empty-branch && test_tick && - git commit --allow-empty -m "empty" + git commit --allow-empty -m "empty" && + + echo third >> file1 && + git add file1 && + test_tick && + git commit --allow-empty-message -m "" ' test_expect_success 'cherry-pick an empty commit' ' + git checkout master && { + git cherry-pick empty-branch^ + test "$?" = 1 + } +' + +test_expect_success 'index lockfile was removed' ' + + test ! -f .git/index.lock + +' + +test_expect_success 'cherry-pick a commit with an empty message' ' git checkout master && { git cherry-pick empty-branch test "$?" = 1 From 11af2aaed657d10dea083f5d5cb7f93bb96a7b70 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Thu, 22 Jul 2010 15:18:30 +0200 Subject: [PATCH 2/6] revert: refactor code to find commit subject in find_commit_subject() Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- builtin/revert.c | 14 ++------------ commit.c | 19 +++++++++++++++++++ commit.h | 3 +++ 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/builtin/revert.c b/builtin/revert.c index c170715ba8..d47794495a 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -98,7 +98,7 @@ struct commit_message { static int get_message(const char *raw_message, struct commit_message *out) { const char *encoding; - const char *p, *abbrev, *eol; + const char *p, *abbrev; char *q; int abbrev_len, oneline_len; @@ -121,17 +121,7 @@ static int get_message(const char *raw_message, struct commit_message *out) abbrev = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV); abbrev_len = strlen(abbrev); - /* Find beginning and end of commit subject. */ - p = out->message; - while (*p && (*p != '\n' || p[1] != '\n')) - p++; - if (*p) { - p += 2; - for (eol = p; *eol && *eol != '\n'; eol++) - ; /* do nothing */ - } else - eol = p; - oneline_len = eol - p; + oneline_len = find_commit_subject(out->message, &p); out->parent_label = xmalloc(strlen("parent of ") + abbrev_len + strlen("... ") + oneline_len + 1); diff --git a/commit.c b/commit.c index 731191e63b..fa1a5bac30 100644 --- a/commit.c +++ b/commit.c @@ -315,6 +315,25 @@ int parse_commit(struct commit *item) return ret; } +int find_commit_subject(const char *commit_buffer, const char **subject) +{ + const char *eol; + const char *p = commit_buffer; + + while (*p && (*p != '\n' || p[1] != '\n')) + p++; + if (*p) { + p += 2; + for (eol = p; *eol && *eol != '\n'; eol++) + ; /* do nothing */ + } else + eol = p; + + *subject = p; + + return eol - p; +} + struct commit_list *commit_list_insert(struct commit *item, struct commit_list **list_p) { struct commit_list *new_list = xmalloc(sizeof(struct commit_list)); diff --git a/commit.h b/commit.h index 26ec8c0d1c..236cf1333a 100644 --- a/commit.h +++ b/commit.h @@ -40,6 +40,9 @@ int parse_commit_buffer(struct commit *item, void *buffer, unsigned long size); int parse_commit(struct commit *item); +/* Find beginning and length of commit subject. */ +int find_commit_subject(const char *commit_buffer, const char **subject); + struct commit_list * commit_list_insert(struct commit *item, struct commit_list **list_p); unsigned commit_list_count(const struct commit_list *l); struct commit_list * insert_by_date(struct commit *item, struct commit_list **list); From dfe7effe7d873015c8624d438b98671083c12e27 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Thu, 22 Jul 2010 15:18:31 +0200 Subject: [PATCH 3/6] revert: rename variables related to subject in get_message() Generic-looking pointer variable "p" was used only to point at subject string and had a rather lifespan. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- builtin/revert.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/builtin/revert.c b/builtin/revert.c index d47794495a..6d1b9ca1a5 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -98,9 +98,9 @@ struct commit_message { static int get_message(const char *raw_message, struct commit_message *out) { const char *encoding; - const char *p, *abbrev; + const char *abbrev, *subject; + int abbrev_len, subject_len; char *q; - int abbrev_len, oneline_len; if (!raw_message) return -1; @@ -121,17 +121,17 @@ static int get_message(const char *raw_message, struct commit_message *out) abbrev = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV); abbrev_len = strlen(abbrev); - oneline_len = find_commit_subject(out->message, &p); + subject_len = find_commit_subject(out->message, &subject); out->parent_label = xmalloc(strlen("parent of ") + abbrev_len + - strlen("... ") + oneline_len + 1); + strlen("... ") + subject_len + 1); q = out->parent_label; q = mempcpy(q, "parent of ", strlen("parent of ")); out->label = q; q = mempcpy(q, abbrev, abbrev_len); q = mempcpy(q, "... ", strlen("... ")); out->subject = q; - q = mempcpy(q, p, oneline_len); + q = mempcpy(q, subject, subject_len); *q = '\0'; return 0; } From 56ff37941eb76d3f0097884715ca8b391788a3c9 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Thu, 22 Jul 2010 15:18:33 +0200 Subject: [PATCH 4/6] bisect: use find_commit_subject() instead of custom code Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- bisect.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/bisect.c b/bisect.c index b556b11610..060c042f8b 100644 --- a/bisect.c +++ b/bisect.c @@ -141,7 +141,8 @@ static void show_list(const char *debug, int counted, int nr, enum object_type type; unsigned long size; char *buf = read_sha1_file(commit->object.sha1, &type, &size); - char *ep, *sp; + const char *subject_start; + int subject_len; fprintf(stderr, "%c%c%c ", (flags & TREESAME) ? ' ' : 'T', @@ -156,13 +157,9 @@ static void show_list(const char *debug, int counted, int nr, fprintf(stderr, " %.*s", 8, sha1_to_hex(pp->item->object.sha1)); - sp = strstr(buf, "\n\n"); - if (sp) { - sp += 2; - for (ep = sp; *ep && *ep != '\n'; ep++) - ; - fprintf(stderr, " %.*s", (int)(ep - sp), sp); - } + subject_len = find_commit_subject(buf, &subject_start); + if (subject_len) + fprintf(stderr, " %.*s", subject_len, subject_start); fprintf(stderr, "\n"); } } From 49b7120ef1c915439b01f23f30ddfe64565ec9cc Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Thu, 22 Jul 2010 15:18:34 +0200 Subject: [PATCH 5/6] merge-recursive: use find_commit_subject() instead of custom code Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- merge-recursive.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/merge-recursive.c b/merge-recursive.c index 206c103635..34837bd5d4 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -136,16 +136,10 @@ static void output_commit_title(struct merge_options *o, struct commit *commit) if (parse_commit(commit) != 0) printf("(bad commit)\n"); else { - const char *s; - int len; - for (s = commit->buffer; *s; s++) - if (*s == '\n' && s[1] == '\n') { - s += 2; - break; - } - for (len = 0; s[len] && '\n' != s[len]; len++) - ; /* do nothing */ - printf("%.*s\n", len, s); + const char *title; + int len = find_commit_subject(commit->buffer, &title); + if (len) + printf("%.*s\n", len, title); } } } From ad98a58b3d7a151dca59364b72097b6b875a56f6 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Thu, 22 Jul 2010 15:18:35 +0200 Subject: [PATCH 6/6] blame: use find_commit_subject() instead of custom code Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- builtin/blame.c | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/builtin/blame.c b/builtin/blame.c index 8506286dd2..8dca385f1f 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -1371,7 +1371,8 @@ static void get_commit_info(struct commit *commit, int detailed) { int len; - char *tmp, *endp, *reencoded, *message; + const char *subject; + char *reencoded, *message; static char author_name[1024]; static char author_mail[1024]; static char committer_name[1024]; @@ -1413,22 +1414,13 @@ static void get_commit_info(struct commit *commit, &ret->committer_time, &ret->committer_tz); ret->summary = summary_buf; - tmp = strstr(message, "\n\n"); - if (!tmp) { - error_out: + len = find_commit_subject(message, &subject); + if (len && len < sizeof(summary_buf)) { + memcpy(summary_buf, subject, len); + summary_buf[len] = 0; + } else { sprintf(summary_buf, "(%s)", sha1_to_hex(commit->object.sha1)); - free(reencoded); - return; } - tmp += 2; - endp = strchr(tmp, '\n'); - if (!endp) - endp = tmp + strlen(tmp); - len = endp - tmp; - if (len >= sizeof(summary_buf) || len == 0) - goto error_out; - memcpy(summary_buf, tmp, len); - summary_buf[len] = 0; free(reencoded); }