git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 03/18] revert: Simplify and inline add_message_to_msg

From
Ramkumar Ramachandra <artagnon@gmail.com>
Date
Jul 28, 2011, 16:52 UTC
Message-ID
<1311871951-3497-4-git-send-email-artagnon@gmail.com>
In-Reply-To
<1311871951-3497-1-git-send-email-artagnon@gmail.com>

The add_message_to_msg function has some dead code, an unclear API, only one callsite. While it originally intended fill up an empty commit message with the commit object name while picking, it really doesn't do this -- a bug introduced in v1.5.1-rc1~65^2~2 (Make git-revert & git-cherry-pick a builtin, 2007-03-01). Today, tests in t3505-cherry-pick-empty.sh indicate that not filling up an empty commit message is the desired behavior. Re-implement and inline the function accordingly, with a benefecial side-effect: don't dereference a NULL pointer when the commit doesn't have a delimeter after the header.

Helped-by: Junio C Hamano <gitster@pobox.com>
Mentored-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 builtin/revert.c |   28 ++++++++++++++--------------
 1 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 2df3f3b..7dfe295 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -185,19 +185,6 @@ static char *get_encoding(const char *message)
 	return NULL;
 }
 
-static void add_message_to_msg(struct strbuf *msgbuf, const char *message)
-{
-	const char *p = message;
-	while (*p && (*p != '\n' || p[1] != '\n'))
-		p++;
-
-	if (!*p)
-		strbuf_addstr(msgbuf, sha1_to_hex(commit->object.sha1));
-
-	p += 2;
-	strbuf_addstr(msgbuf, p);
-}
-
 static void write_cherry_pick_head(void)
 {
 	int fd;
@@ -462,11 +449,24 @@ static int do_pick_commit(void)
 		}
 		strbuf_addstr(&msgbuf, ".\n");
 	} else {
+		const char *p;
+
 		base = parent;
 		base_label = msg.parent_label;
 		next = commit;
 		next_label = msg.label;
-		add_message_to_msg(&msgbuf, msg.message);
+
+		/*
+		 * Append the commit log message to msgbuf; it starts
+		 * after the tree, parent, author, committer
+		 * information followed by "\n\n".
+		 */
+		p = strstr(msg.message, "\n\n");
+		if (p) {
+			p += 2;
+			strbuf_addstr(&msgbuf, p);
+		}
+
 		if (no_replay) {
 			strbuf_addstr(&msgbuf, "(cherry picked from commit ");
 			strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
-- 
1.7.4.rc1.7.g2cf08.dirty
Previous: Ramkumar RamachandraNext: Ramkumar Ramachandra
Message 6 of 36 in “Sequencer for inclusion v4”
  1. 00/18 Sequencer for inclusion v4Ramkumar Ramachandra, Jul 28, 2011
  2. 01/18 advice: Introduce error_resolve_conflictRamkumar Ramachandra, Jul 28, 2011
  3. 02/18 config: Introduce functions to write non-standard fileRamkumar Ramachandra, Jul 28, 2011
  4. Jonathan NiederJul 28, 2011
  5. Ramkumar RamachandraJul 28, 2011
  6. 03/18 revert: Simplify and inline add_message_to_msgRamkumar Ramachandra, Jul 28, 2011
  7. 04/18 revert: Don't check lone argument in get_encodingRamkumar Ramachandra, Jul 28, 2011
  8. 05/18 revert: Rename no_replay to record_originRamkumar Ramachandra, Jul 28, 2011
  9. 06/18 revert: Eliminate global "commit" variableRamkumar Ramachandra, Jul 28, 2011
  10. 07/18 revert: Introduce struct to keep command-line optionsRamkumar Ramachandra, Jul 28, 2011
  11. 08/18 revert: Separate cmdline parsing from functional codeRamkumar Ramachandra, Jul 28, 2011
  12. 09/18 revert: Don't create invalid replay_opts in parse_argsRamkumar Ramachandra, Jul 28, 2011
  13. 10/18 revert: Save data for continuing after conflict resolutionRamkumar Ramachandra, Jul 28, 2011
  14. Jonathan NiederJul 28, 2011
  15. Ramkumar RamachandraJul 28, 2011
  16. 11/18 revert: Save command-line options for continuing operationRamkumar Ramachandra, Jul 28, 2011
  17. Christian CouderJul 31, 2011
  18. Ramkumar RamachandraAug 1, 2011
  19. 12/18 revert: Make pick_commits functionally act on a commit listRamkumar Ramachandra, Jul 28, 2011
  20. Christian CouderJul 31, 2011
  21. Christian CouderJul 31, 2011
  22. Ramkumar RamachandraAug 1, 2011
  23. 13/18 revert: Introduce --reset to remove sequencer stateRamkumar Ramachandra, Jul 28, 2011
  24. Christian CouderJul 31, 2011
  25. Ramkumar RamachandraAug 1, 2011
  26. 14/18 reset: Make reset remove the sequencer stateRamkumar Ramachandra, Jul 28, 2011
  27. 15/18 revert: Remove sequencer state when no commits are pendingRamkumar Ramachandra, Jul 28, 2011
  28. 16/18 revert: Don't implictly stomp pending sequencer operationRamkumar Ramachandra, Jul 28, 2011
  29. 17/18 revert: Introduce --continue to continue the operationRamkumar Ramachandra, Jul 28, 2011
  30. Christian CouderJul 31, 2011
  31. Ramkumar RamachandraAug 1, 2011
  32. Jonathan NiederAug 3, 2011
  33. Junio C HamanoAug 3, 2011
  34. Ramkumar RamachandraAug 3, 2011
  35. 18/18 revert: Propogate errors upwards from do_pick_commitRamkumar Ramachandra, Jul 28, 2011
  36. Ramkumar RamachandraJul 28, 2011

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.