git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <junkio@cox.net>
Cc: git@vger.kernel.org
Subject: [PATCH] commit: fix pretty-printing of messages with "\nencoding "
Date: Wed, 28 Mar 2007 17:52:09 -0400	[thread overview]
Message-ID: <20070328215209.GA13672@coredump.intra.peff.net> (raw)

The function replace_encoding_header is given the whole
commit buffer, including the commit message. When looking
for the encoding header, if none was found in the header, it
would locate any line in the commit message matching
"\nencoding " and remove it.

Instead, we now make sure to search only to the end of the
header.

Signed-off-by: Jeff King <peff@peff.net>
---
You can see the bug by doing this:
  git-commit -m 'encoding foo'
  git-show
and getting a blank log message (unless, of course, you're using a
non-utf8 commit encoding, in which case you will actually have an
encoding header).

I wonder, though, if this function before or after is actually correct;
if there is no encoding header, we exit the function immediately. But if
we are changing the encoding from utf8 to a non-utf8 value, we
presumably should continue and actually insert the new encoding header.

The searching could potentially be refactored to share code with
get_header; however, either the interface gets a little hairy, or you
have to repeat a few strlen()s, and I seem to recall a recent effort to
reduce such calls in critical paths (which I think this probably is).
The implementation here isn't ugly, anyway.

 commit.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/commit.c b/commit.c
index a4f2e74..754d1b8 100644
--- a/commit.c
+++ b/commit.c
@@ -654,6 +654,7 @@ static char *get_header(const struct commit *commit, const char *key)
 static char *replace_encoding_header(char *buf, const char *encoding)
 {
 	char *encoding_header = strstr(buf, "\nencoding ");
+	char *header_end = strstr(buf, "\n\n");
 	char *end_of_encoding_header;
 	int encoding_header_pos;
 	int encoding_header_len;
@@ -661,8 +662,10 @@ static char *replace_encoding_header(char *buf, const char *encoding)
 	int need_len;
 	int buflen = strlen(buf) + 1;
 
-	if (!encoding_header)
-		return buf; /* should not happen but be defensive */
+	if (!header_end)
+		header_end = buf + buflen;
+	if (!encoding_header || encoding_header >= header_end)
+		return buf;
 	encoding_header++;
 	end_of_encoding_header = strchr(encoding_header, '\n');
 	if (!end_of_encoding_header)
-- 
1.5.1.rc2.636.g7ca6fa-dirty

             reply	other threads:[~2007-03-28 21:52 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-03-28 21:52 Jeff King [this message]
2007-03-28 22:10 ` [PATCH] commit: fix pretty-printing of messages with "\nencoding " Junio C Hamano
2007-03-28 22:15   ` 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=20070328215209.GA13672@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=junkio@cox.net \
    /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).