{"thread":{"id":"7458","subject":"[PATCH] commit: fix pretty-printing of messages with \"\\nencoding \"","startedAt":"2007-03-28T21:52:09Z","lastAt":"2007-03-28T22:15:17Z","messageCount":3,"participants":["Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"38270","messageId":"20070328215209.GA13672@coredump.intra.peff.net","threadId":"7458","inReplyTo":null,"subject":"[PATCH] commit: fix pretty-printing of messages with \"\\nencoding \"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-03-28T21:52:09Z","receivedAt":"2007-03-28T21:52:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The function replace_encoding_header is given the whole\ncommit buffer, including the commit message. When looking\nfor the encoding header, if none was found in the header, it\nwould locate any line in the commit message matching\n\"\\nencoding \" and remove it.\n\nInstead, we now make sure to search only to the end of the\nheader.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nYou can see the bug by doing this:\n  git-commit -m 'encoding foo'\n  git-show\nand getting a blank log message (unless, of course, you're using a\nnon-utf8 commit encoding, in which case you will actually have an\nencoding header).\n\nI wonder, though, if this function before or after is actually correct;\nif there is no encoding header, we exit the function immediately. But if\nwe are changing the encoding from utf8 to a non-utf8 value, we\npresumably should continue and actually insert the new encoding header.\n\nThe searching could potentially be refactored to share code with\nget_header; however, either the interface gets a little hairy, or you\nhave to repeat a few strlen()s, and I seem to recall a recent effort to\nreduce such calls in critical paths (which I think this probably is).\nThe implementation here isn't ugly, anyway.\n\n commit.c |    7 +++++--\n 1 files changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex a4f2e74..754d1b8 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -654,6 +654,7 @@ static char *get_header(const struct commit *commit, const char *key)\n static char *replace_encoding_header(char *buf, const char *encoding)\n {\n \tchar *encoding_header = strstr(buf, \"\\nencoding \");\n+\tchar *header_end = strstr(buf, \"\\n\\n\");\n \tchar *end_of_encoding_header;\n \tint encoding_header_pos;\n \tint encoding_header_len;\n@@ -661,8 +662,10 @@ static char *replace_encoding_header(char *buf, const char *encoding)\n \tint need_len;\n \tint buflen = strlen(buf) + 1;\n \n-\tif (!encoding_header)\n-\t\treturn buf; /* should not happen but be defensive */\n+\tif (!header_end)\n+\t\theader_end = buf + buflen;\n+\tif (!encoding_header || encoding_header >= header_end)\n+\t\treturn buf;\n \tencoding_header++;\n \tend_of_encoding_header = strchr(encoding_header, '\\n');\n \tif (!end_of_encoding_header)\n-- \n1.5.1.rc2.636.g7ca6fa-dirty\n"},{"id":"38273","messageId":"7v7it1f29s.fsf@assigned-by-dhcp.cox.net","threadId":"7458","inReplyTo":"20070328215209.GA13672@coredump.intra.peff.net","subject":"Re: [PATCH] commit: fix pretty-printing of messages with \"\\nencoding \"","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-28T22:10:07Z","receivedAt":"2007-03-28T22:10:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I wonder, though, if this function before or after is actually correct;\n> if there is no encoding header, we exit the function immediately. But if\n> we are changing the encoding from utf8 to a non-utf8 value, we\n> presumably should continue and actually insert the new encoding header.\n\nThe function is correct; the only reason it may recode to\nnon-utf8 is the user (or Porcelain such as qgit or gitk)\nexplicitly asked to do so -- from the final output they will get\nthe message in user-native encoding and without the extra\nencoding header, thus we retain the backward compatible\nbehaviour before the re-encoding feature was introduced.\n"},{"id":"38275","messageId":"20070328221517.GA25430@coredump.intra.peff.net","threadId":"7458","inReplyTo":"7v7it1f29s.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] commit: fix pretty-printing of messages with \"\\nencoding \"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-03-28T22:15:17Z","receivedAt":"2007-03-28T22:15:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 28, 2007 at 03:10:07PM -0700, Junio C Hamano wrote:\n\n> The function is correct; the only reason it may recode to\n> non-utf8 is the user (or Porcelain such as qgit or gitk)\n> explicitly asked to do so -- from the final output they will get\n> the message in user-native encoding and without the extra\n> encoding header, thus we retain the backward compatible\n> behaviour before the re-encoding feature was introduced.\n\nI see. But then why do we not simply strip the encoding header entirely,\nand return in native format? Why do we insert a new encoding header\n_only_ when there was one previously?\n\n-Peff\n"}]}