git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
	Christian Couder <chriscool@tuxfamily.org>,
	Jakub Narebski <jnareb@gmail.com>,
	Eric Sunshine <sunshine@sunshineco.com>
Subject: [PATCH 13/17] convert logmsg_reencode to get_commit_buffer
Date: Tue, 10 Jun 2014 17:41:39 -0400	[thread overview]
Message-ID: <20140610214139.GM19147@sigill.intra.peff.net> (raw)
In-Reply-To: <20140610213509.GA26979@sigill.intra.peff.net>

Like the callsites in the previous commit, logmsg_reencode
already falls back to read_sha1_file when necessary.
However, I split its conversion out into its own commit
because it's a bit more complex.

We return either:

  1. The original commit->buffer

  2. A newly allocated buffer from read_sha1_file

  3. A reencoded buffer (based on either 1 or 2 above).

while trying to do as few extra reads/allocations as
possible. Callers currently free the result with
logmsg_free, but we can simplify this by pointing them
straight to unuse_commit_buffer. This is a slight layering
violation, in that we may be passing a buffer from (3).
However, since the end result is to free() anything except
(1), which is unlikely to change, and because this makes the
interface much simpler, it's a reasonable bending of the
rules.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/blame.c |  4 ++--
 builtin/reset.c |  2 +-
 commit.h        |  1 -
 pretty.c        | 40 +++++++++++-----------------------------
 revision.c      |  2 +-
 sequencer.c     |  2 +-
 6 files changed, 16 insertions(+), 35 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 0af3a18..cde19eb 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1666,7 +1666,7 @@ static void get_commit_info(struct commit *commit,
 		    &ret->author_time, &ret->author_tz);
 
 	if (!detailed) {
-		logmsg_free(message, commit);
+		unuse_commit_buffer(commit, message);
 		return;
 	}
 
@@ -1680,7 +1680,7 @@ static void get_commit_info(struct commit *commit,
 	else
 		strbuf_addf(&ret->summary, "(%s)", sha1_to_hex(commit->object.sha1));
 
-	logmsg_free(message, commit);
+	unuse_commit_buffer(commit, message);
 }
 
 /*
diff --git a/builtin/reset.c b/builtin/reset.c
index 7ebee07..850d532 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -109,7 +109,7 @@ static void print_new_head_line(struct commit *commit)
 	}
 	else
 		printf("\n");
-	logmsg_free(msg, commit);
+	unuse_commit_buffer(commit, msg);
 }
 
 static void update_index_from_diff(struct diff_queue_struct *q,
diff --git a/commit.h b/commit.h
index 259c0ae..5ce5ce7 100644
--- a/commit.h
+++ b/commit.h
@@ -156,7 +156,6 @@ struct rev_info; /* in revision.h, it circularly uses enum cmit_fmt */
 extern const char *logmsg_reencode(const struct commit *commit,
 				   char **commit_encoding,
 				   const char *output_encoding);
-extern void logmsg_free(const char *msg, const struct commit *commit);
 extern void get_commit_format(const char *arg, struct rev_info *);
 extern const char *format_subject(struct strbuf *sb, const char *msg,
 				  const char *line_separator);
diff --git a/pretty.c b/pretty.c
index d152de2..8fd60cd 100644
--- a/pretty.c
+++ b/pretty.c
@@ -613,22 +613,9 @@ const char *logmsg_reencode(const struct commit *commit,
 	static const char *utf8 = "UTF-8";
 	const char *use_encoding;
 	char *encoding;
-	char *msg = commit->buffer;
+	const char *msg = get_commit_buffer(commit);
 	char *out;
 
-	if (!msg) {
-		enum object_type type;
-		unsigned long size;
-
-		msg = read_sha1_file(commit->object.sha1, &type, &size);
-		if (!msg)
-			die("Cannot read commit object %s",
-			    sha1_to_hex(commit->object.sha1));
-		if (type != OBJ_COMMIT)
-			die("Expected commit for '%s', got %s",
-			    sha1_to_hex(commit->object.sha1), typename(type));
-	}
-
 	if (!output_encoding || !*output_encoding) {
 		if (commit_encoding)
 			*commit_encoding =
@@ -652,12 +639,13 @@ const char *logmsg_reencode(const struct commit *commit,
 		 * Otherwise, we still want to munge the encoding header in the
 		 * result, which will be done by modifying the buffer. If we
 		 * are using a fresh copy, we can reuse it. But if we are using
-		 * the cached copy from commit->buffer, we need to duplicate it
-		 * to avoid munging commit->buffer.
+		 * the cached copy from get_commit_buffer, we need to duplicate it
+		 * to avoid munging the cached copy.
 		 */
-		out = msg;
-		if (out == commit->buffer)
-			out = xstrdup(out);
+		if (msg == get_cached_commit_buffer(commit))
+			out = xstrdup(msg);
+		else
+			out = (char *)msg;
 	}
 	else {
 		/*
@@ -667,8 +655,8 @@ const char *logmsg_reencode(const struct commit *commit,
 		 * copy, we can free it.
 		 */
 		out = reencode_string(msg, output_encoding, use_encoding);
-		if (out && msg != commit->buffer)
-			free(msg);
+		if (out)
+			unuse_commit_buffer(commit, msg);
 	}
 
 	/*
@@ -687,12 +675,6 @@ const char *logmsg_reencode(const struct commit *commit,
 	return out ? out : msg;
 }
 
-void logmsg_free(const char *msg, const struct commit *commit)
-{
-	if (msg != commit->buffer)
-		free((void *)msg);
-}
-
 static int mailmap_name(const char **email, size_t *email_len,
 			const char **name, size_t *name_len)
 {
@@ -1531,7 +1513,7 @@ void format_commit_message(const struct commit *commit,
 	}
 
 	free(context.commit_encoding);
-	logmsg_free(context.message, commit);
+	unuse_commit_buffer(commit, context.message);
 	free(context.signature_check.gpg_output);
 	free(context.signature_check.signer);
 }
@@ -1767,7 +1749,7 @@ void pretty_print_commit(struct pretty_print_context *pp,
 	if (pp->fmt == CMIT_FMT_EMAIL && sb->len <= beginning_of_body)
 		strbuf_addch(sb, '\n');
 
-	logmsg_free(reencoded, commit);
+	unuse_commit_buffer(commit, reencoded);
 }
 
 void pp_commit_easy(enum cmit_fmt fmt, const struct commit *commit,
diff --git a/revision.c b/revision.c
index be151ef..1cc91e5 100644
--- a/revision.c
+++ b/revision.c
@@ -2844,7 +2844,7 @@ static int commit_match(struct commit *commit, struct rev_info *opt)
 		retval = grep_buffer(&opt->grep_filter,
 				     (char *)message, strlen(message));
 	strbuf_release(&buf);
-	logmsg_free(message, commit);
+	unuse_commit_buffer(commit, message);
 	return retval;
 }
 
diff --git a/sequencer.c b/sequencer.c
index 3fcab4d..4632f7d 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -154,7 +154,7 @@ static int get_message(struct commit *commit, struct commit_message *out)
 static void free_message(struct commit *commit, struct commit_message *msg)
 {
 	free(msg->parent_label);
-	logmsg_free(msg->message, commit);
+	unuse_commit_buffer(commit, msg->message);
 }
 
 static void write_cherry_pick_head(struct commit *commit, const char *pseudoref)
-- 
2.0.0.729.g520999f

  parent reply	other threads:[~2014-06-10 21:41 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-09 18:02 [PATCH 0/15] store length of commit->buffer Jeff King
2014-06-09 18:09 ` [PATCH 01/15] alloc: include any-object allocations in alloc_report Jeff King
2014-06-09 18:09 ` [PATCH 02/15] commit: push commit_index update into alloc_commit_node Jeff King
2014-06-10 21:31   ` Junio C Hamano
2014-06-09 18:10 ` [PATCH 03/15] do not create "struct commit" with xcalloc Jeff King
2014-06-10 21:35   ` Junio C Hamano
2014-06-10 21:49     ` Jeff King
2014-06-09 18:10 ` [PATCH 04/15] logmsg_reencode: return const buffer Jeff King
2014-06-10  1:36   ` Eric Sunshine
2014-06-09 18:10 ` [PATCH 05/15] sequencer: use logmsg_reencode in get_message Jeff King
2014-06-10 21:40   ` Junio C Hamano
2014-06-10 21:50     ` Jeff King
2014-06-09 18:11 ` [PATCH 06/15] provide a helper to free commit buffer Jeff King
2014-06-09 18:11 ` [PATCH 07/15] provide a helper to set the " Jeff King
2014-06-09 18:11 ` [PATCH 08/15] provide helpers to access " Jeff King
2014-06-10  8:00   ` Eric Sunshine
2014-06-09 18:12 ` [PATCH 09/15] use get_cached_commit_buffer where appropriate Jeff King
2014-06-09 18:12 ` [PATCH 10/15] use get_commit_buffer to avoid duplicate code Jeff King
2014-06-10  8:01   ` Eric Sunshine
2014-06-10 20:34     ` Jeff King
2014-06-09 18:13 ` [PATCH 11/15] convert logmsg_reencode to get_commit_buffer Jeff King
2014-06-09 18:13 ` [PATCH 12/15] use get_commit_buffer everywhere Jeff King
2014-06-09 22:40   ` Junio C Hamano
2014-06-10  0:02     ` Jeff King
2014-06-10  0:22       ` Jeff King
2014-06-10 16:06       ` Junio C Hamano
2014-06-10 21:27         ` Jeff King
2014-06-09 18:15 ` [PATCH 13/15] commit-slab: provide a static initializer Jeff King
2014-06-09 18:15 ` [PATCH 14/15] commit: convert commit->buffer to a slab Jeff King
2014-06-09 18:15 ` [PATCH 15/15] commit: record buffer length in cache Jeff King
2014-06-10  5:12   ` Christian Couder
2014-06-10  5:27     ` Jeff King
2014-06-10 20:33       ` Jeff King
2014-06-10 21:30         ` Christian Couder
2014-06-10 21:35 ` [PATCH v2 0/17] store length of commit->buffer Jeff King
2014-06-10 21:36   ` [PATCH 01/17] commit_tree: take a pointer/len pair rather than a const strbuf Jeff King
2014-06-10 21:38   ` [PATCH 02/17] replace dangerous uses of strbuf_attach Jeff King
2014-06-10 21:38   ` [PATCH 03/17] alloc: include any-object allocations in alloc_report Jeff King
2014-06-10 21:39   ` [PATCH 04/17] commit: push commit_index update into alloc_commit_node Jeff King
2014-06-10 21:39   ` [PATCH 05/17] do not create "struct commit" with xcalloc Jeff King
2014-06-10 21:39   ` [PATCH 06/17] logmsg_reencode: return const buffer Jeff King
2014-06-10 21:39   ` [PATCH 07/17] sequencer: use logmsg_reencode in get_message Jeff King
2014-06-10 21:40   ` [PATCH 08/17] provide a helper to free commit buffer Jeff King
2014-06-12 18:22     ` Junio C Hamano
2014-06-12 20:08       ` Jeff King
2014-06-12 22:05         ` Jeff King
2014-06-10 21:40   ` [PATCH 09/17] provide a helper to set the " Jeff King
2014-06-10 21:40   ` [PATCH 10/17] provide helpers to access " Jeff King
2014-06-10 21:40   ` [PATCH 11/17] use get_cached_commit_buffer where appropriate Jeff King
2014-06-10 21:41   ` [PATCH 12/17] use get_commit_buffer to avoid duplicate code Jeff King
2014-06-10 21:41   ` Jeff King [this message]
2014-06-10 21:41   ` [PATCH 14/17] use get_commit_buffer everywhere Jeff King
2014-06-10 21:42   ` [PATCH 15/17] commit-slab: provide a static initializer Jeff King
2014-06-12 18:15     ` Junio C Hamano
2014-06-12 19:51       ` Jeff King
2014-06-10 21:43   ` [PATCH 16/17] commit: convert commit->buffer to a slab Jeff King
2014-06-10 21:44   ` [PATCH 17/17] commit: record buffer length in cache Jeff King
2014-06-10 21:46   ` [PATCH v2 0/17] store length of commit->buffer Jeff King
2014-06-12 17:22     ` Junio C Hamano
2014-06-13  6:32   ` [PATCH v2] reuse cached commit buffer when parsing signatures Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140610214139.GM19147@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=chriscool@tuxfamily.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jnareb@gmail.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).