From bf3de2b373d4fa55b6040c7dc6f7f8668ef45c19 Mon Sep 17 00:00:00 2001 From: Ramkumar Ramachandra Date: Wed, 14 Dec 2011 22:24:28 +0530 Subject: revert: free msg in format_todo() Memory allocated to the fields of msg by get_message() isn't freed. This is potentially a big leak, because fresh memory is allocated to store the commit message for each commit. Fix this using free_message(). Reported-by: Jonathan Nieder Signed-off-by: Ramkumar Ramachandra Signed-off-by: Junio C Hamano --- builtin/revert.c | 1 + 1 file changed, 1 insertion(+) (limited to 'builtin') diff --git a/builtin/revert.c b/builtin/revert.c index 028bcbcd75..76a1633b9b 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -709,6 +709,7 @@ static int format_todo(struct strbuf *buf, struct commit_list *todo_list, if (get_message(cur->item, &msg)) return error(_("Cannot get commit message for %s"), sha1_abbrev); strbuf_addf(buf, "%s %s %s\n", action_str, sha1_abbrev, msg.subject); + free_message(&msg); } return 0; } -- cgit v1.2.3 From 6bc1a235b1260a8395045261f8004ba9f12677c7 Mon Sep 17 00:00:00 2001 From: Ramkumar Ramachandra Date: Wed, 14 Dec 2011 22:24:29 +0530 Subject: revert: make commit subjects in insn sheet optional Change the instruction sheet format subtly so that the subject of the commit message that follows the object name is optional. As a result, an instruction sheet like this is now perfectly valid: pick 35b0426 pick fbd5bbcbc2e pick 7362160f While at it, also fix a bug introduced by 5a5d80f4 (revert: Introduce --continue to continue the operation, 2011-08-04) that failed to read lines that are too long to fit on the commit-id-shaped buffer we currently use; eliminate the need for the buffer altogether. In addition to literal SHA-1 hexes, you can now safely use expressions like the following in the instruction sheet: featurebranch~4 rr/revert-cherry-pick-continue^2~12@{12 days ago} [jc: simplify parsing] Suggested-by: Jonathan Nieder Helped-by: Junio C Hamano Signed-off-by: Ramkumar Ramachandra Signed-off-by: Junio C Hamano --- builtin/revert.c | 37 ++++++++++++++++--------------------- 1 file changed, 16 insertions(+), 21 deletions(-) (limited to 'builtin') diff --git a/builtin/revert.c b/builtin/revert.c index 76a1633b9b..6d520aee7d 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -714,31 +714,27 @@ static int format_todo(struct strbuf *buf, struct commit_list *todo_list, return 0; } -static struct commit *parse_insn_line(char *start, struct replay_opts *opts) +static struct commit *parse_insn_line(char *bol, char *eol, struct replay_opts *opts) { unsigned char commit_sha1[20]; - char sha1_abbrev[40]; enum replay_action action; - int insn_len = 0; - char *p, *q; + char *end_of_object_name; + int saved, status; - if (!prefixcmp(start, "pick ")) { + if (!prefixcmp(bol, "pick ")) { action = CHERRY_PICK; - insn_len = strlen("pick"); - p = start + insn_len + 1; - } else if (!prefixcmp(start, "revert ")) { + bol += strlen("pick "); + } else if (!prefixcmp(bol, "revert ")) { action = REVERT; - insn_len = strlen("revert"); - p = start + insn_len + 1; + bol += strlen("revert "); } else return NULL; - q = strchr(p, ' '); - if (!q) - return NULL; - q++; - - strlcpy(sha1_abbrev, p, q - p); + end_of_object_name = bol + strcspn(bol, " \n"); + saved = *end_of_object_name; + *end_of_object_name = '\0'; + status = get_sha1(bol, commit_sha1); + *end_of_object_name = saved; /* * Verify that the action matches up with the one in @@ -751,7 +747,7 @@ static struct commit *parse_insn_line(char *start, struct replay_opts *opts) return NULL; } - if (get_sha1(sha1_abbrev, commit_sha1) < 0) + if (status < 0) return NULL; return lookup_commit_reference(commit_sha1); @@ -766,13 +762,12 @@ static int parse_insn_buffer(char *buf, struct commit_list **todo_list, int i; for (i = 1; *p; i++) { - commit = parse_insn_line(p, opts); + char *eol = strchrnul(p, '\n'); + commit = parse_insn_line(p, eol, opts); if (!commit) return error(_("Could not parse line %d."), i); next = commit_list_append(commit, next); - p = strchrnul(p, '\n'); - if (*p) - p++; + p = *eol ? eol + 1 : eol; } if (!*todo_list) return error(_("No commits parsed.")); -- cgit v1.2.3 From 0db76962d156c196a23a98014c1585246f561a5d Mon Sep 17 00:00:00 2001 From: Ramkumar Ramachandra Date: Wed, 14 Dec 2011 22:24:30 +0530 Subject: revert: tolerate extra spaces, tabs in insn sheet Tolerate extra spaces and tabs as part of the the field separator in '.git/sequencer/todo', for people with fat fingers. Suggested-by: Junio C Hamano Signed-off-by: Ramkumar Ramachandra Signed-off-by: Junio C Hamano --- builtin/revert.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) (limited to 'builtin') diff --git a/builtin/revert.c b/builtin/revert.c index 6d520aee7d..164552e05a 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -719,18 +719,24 @@ static struct commit *parse_insn_line(char *bol, char *eol, struct replay_opts * unsigned char commit_sha1[20]; enum replay_action action; char *end_of_object_name; - int saved, status; + int saved, status, padding; - if (!prefixcmp(bol, "pick ")) { + if (!prefixcmp(bol, "pick")) { action = CHERRY_PICK; - bol += strlen("pick "); - } else if (!prefixcmp(bol, "revert ")) { + bol += strlen("pick"); + } else if (!prefixcmp(bol, "revert")) { action = REVERT; - bol += strlen("revert "); + bol += strlen("revert"); } else return NULL; - end_of_object_name = bol + strcspn(bol, " \n"); + /* Eat up extra spaces/ tabs before object name */ + padding = strspn(bol, " \t"); + if (!padding) + return NULL; + bol += padding; + + end_of_object_name = bol + strcspn(bol, " \t\n"); saved = *end_of_object_name; *end_of_object_name = '\0'; status = get_sha1(bol, commit_sha1); -- cgit v1.2.3 From 9e1313648d4c0ee1bab0ee3d7ed22553bd5bf87c Mon Sep 17 00:00:00 2001 From: Ramkumar Ramachandra Date: Wed, 14 Dec 2011 22:24:31 +0530 Subject: revert: simplify getting commit subject in format_todo() format_todo() calls get_message(), but uses only the subject line of the commit message. As a minor optimization, save work and unnecessary memory allocations by using find_commit_subject() instead. Also, remove the unnecessary check on cur->item->buffer: the lookup_commit_reference() call in parse_insn_line() has already made sure of this. Suggested-by: Jonathan Nieder Helped-by: Junio C Hamano Signed-off-by: Ramkumar Ramachandra Signed-off-by: Junio C Hamano --- builtin/revert.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'builtin') diff --git a/builtin/revert.c b/builtin/revert.c index 164552e05a..0a86fecbd0 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -700,16 +700,16 @@ static int format_todo(struct strbuf *buf, struct commit_list *todo_list, struct replay_opts *opts) { struct commit_list *cur = NULL; - struct commit_message msg = { NULL, NULL, NULL, NULL, NULL }; const char *sha1_abbrev = NULL; const char *action_str = opts->action == REVERT ? "revert" : "pick"; + const char *subject; + int subject_len; for (cur = todo_list; cur; cur = cur->next) { sha1_abbrev = find_unique_abbrev(cur->item->object.sha1, DEFAULT_ABBREV); - if (get_message(cur->item, &msg)) - return error(_("Cannot get commit message for %s"), sha1_abbrev); - strbuf_addf(buf, "%s %s %s\n", action_str, sha1_abbrev, msg.subject); - free_message(&msg); + subject_len = find_commit_subject(cur->item->buffer, &subject); + strbuf_addf(buf, "%s %s %.*s\n", action_str, sha1_abbrev, + subject_len, subject); } return 0; } -- cgit v1.2.3