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
Aug 4, 2011, 10:39 UTC
Message-ID
<1312454356-3070-4-git-send-email-artagnon@gmail.com>
In-Reply-To
<1312454356-3070-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 beneficial 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.6.351.gb35ac.dirty
Previous: Ramkumar RamachandraNext: Ramkumar Ramachandra
Message 4 of 25 in “Sequencer for inclusion v6”
  1. 00/18 Sequencer for inclusion v6Ramkumar Ramachandra, Aug 4, 2011
  2. 01/18 advice: Introduce error_resolve_conflictRamkumar Ramachandra, Aug 4, 2011
  3. 02/18 config: Introduce functions to write non-standard fileRamkumar Ramachandra, Aug 4, 2011
  4. 03/18 revert: Simplify and inline add_message_to_msgRamkumar Ramachandra, Aug 4, 2011
  5. 04/18 revert: Don't check lone argument in get_encodingRamkumar Ramachandra, Aug 4, 2011
  6. 05/18 revert: Rename no_replay to record_originRamkumar Ramachandra, Aug 4, 2011
  7. 06/18 revert: Eliminate global "commit" variableRamkumar Ramachandra, Aug 4, 2011
  8. 07/18 revert: Introduce struct to keep command-line optionsRamkumar Ramachandra, Aug 4, 2011
  9. 08/18 revert: Separate cmdline parsing from functional codeRamkumar Ramachandra, Aug 4, 2011
  10. 09/18 revert: Don't create invalid replay_opts in parse_argsRamkumar Ramachandra, Aug 4, 2011
  11. 10/18 revert: Save data for continuing after conflict resolutionRamkumar Ramachandra, Aug 4, 2011
  12. 11/18 revert: Save command-line options for continuing operationRamkumar Ramachandra, Aug 4, 2011
  13. 12/18 revert: Make pick_commits functionally act on a commit listRamkumar Ramachandra, Aug 4, 2011
  14. 13/18 revert: Introduce --reset to remove sequencer stateRamkumar Ramachandra, Aug 4, 2011
  15. 14/18 reset: Make reset remove the sequencer stateRamkumar Ramachandra, Aug 4, 2011
  16. Christian CouderAug 8, 2011
  17. Ramkumar RamachandraAug 8, 2011
  18. 15/18 revert: Remove sequencer state when no commits are pendingRamkumar Ramachandra, Aug 4, 2011
  19. 16/18 revert: Don't implicitly stomp pending sequencer operationRamkumar Ramachandra, Aug 4, 2011
  20. 17/18 revert: Introduce --continue to continue the operationRamkumar Ramachandra, Aug 4, 2011
  21. Christian CouderAug 8, 2011
  22. Ramkumar RamachandraAug 8, 2011
  23. Junio C HamanoAug 8, 2011
  24. Ramkumar RamachandraAug 9, 2011
  25. 18/18 revert: Propagate errors upwards from do_pick_commitRamkumar Ramachandra, Aug 4, 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.