From b000c59b0c80fc187e5e0e48dc9396cd60576c4e Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 10 Jun 2014 17:39:30 -0400 Subject: logmsg_reencode: return const buffer The return value from logmsg_reencode may be either a newly allocated buffer or a pointer to the existing commit->buffer. We would not want the caller to accidentally free() or modify the latter, so let's mark it as const. We can cast away the constness in logmsg_free, but only once we have determined that it is a free-able buffer. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- pretty.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'pretty.c') diff --git a/pretty.c b/pretty.c index 3c43db558a..85b0bb3431 100644 --- a/pretty.c +++ b/pretty.c @@ -606,9 +606,9 @@ static char *replace_encoding_header(char *buf, const char *encoding) return strbuf_detach(&tmp, NULL); } -char *logmsg_reencode(const struct commit *commit, - char **commit_encoding, - const char *output_encoding) +const char *logmsg_reencode(const struct commit *commit, + char **commit_encoding, + const char *output_encoding) { static const char *utf8 = "UTF-8"; const char *use_encoding; @@ -687,10 +687,10 @@ char *logmsg_reencode(const struct commit *commit, return out ? out : msg; } -void logmsg_free(char *msg, const struct commit *commit) +void logmsg_free(const char *msg, const struct commit *commit) { if (msg != commit->buffer) - free(msg); + free((void *)msg); } static int mailmap_name(const char **email, size_t *email_len, @@ -796,7 +796,7 @@ struct format_commit_context { struct signature_check signature_check; enum flush_type flush_type; enum trunc_type truncate; - char *message; + const char *message; char *commit_encoding; size_t width, indent1, indent2; int auto_color; @@ -1700,7 +1700,7 @@ void pretty_print_commit(struct pretty_print_context *pp, unsigned long beginning_of_body; int indent = 4; const char *msg; - char *reencoded; + const char *reencoded; const char *encoding; int need_8bit_cte = pp->need_8bit_cte; -- cgit v1.2.1 From b66103c3baa593a39b8b0751213b9fce60e94de4 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 10 Jun 2014 17:41:39 -0400 Subject: convert logmsg_reencode to get_commit_buffer 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 Signed-off-by: Junio C Hamano --- pretty.c | 40 +++++++++++----------------------------- 1 file changed, 11 insertions(+), 29 deletions(-) (limited to 'pretty.c') diff --git a/pretty.c b/pretty.c index 85b0bb3431..915bd1e2e9 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, -- cgit v1.2.1 From 8597ea3afea067b39ba7d4adae7ec6c1ee0e7c91 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 10 Jun 2014 17:44:13 -0400 Subject: commit: record buffer length in cache Most callsites which use the commit buffer try to use the cached version attached to the commit, rather than re-reading from disk. Unfortunately, that interface provides only a pointer to the NUL-terminated buffer, with no indication of the original length. For the most part, this doesn't matter. People do not put NULs in their commit messages, and the log code is happy to treat it all as a NUL-terminated string. However, some code paths do care. For example, when checking signatures, we want to be very careful that we verify all the bytes to avoid malicious trickery. This patch just adds an optional "size" out-pointer to get_commit_buffer and friends. The existing callers all pass NULL (there did not seem to be any obvious sites where we could avoid an immediate strlen() call, though perhaps with some further refactoring we could). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- pretty.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'pretty.c') diff --git a/pretty.c b/pretty.c index 915bd1e2e9..b9fceedbb9 100644 --- a/pretty.c +++ b/pretty.c @@ -613,7 +613,7 @@ const char *logmsg_reencode(const struct commit *commit, static const char *utf8 = "UTF-8"; const char *use_encoding; char *encoding; - const char *msg = get_commit_buffer(commit); + const char *msg = get_commit_buffer(commit, NULL); char *out; if (!output_encoding || !*output_encoding) { @@ -642,7 +642,7 @@ const char *logmsg_reencode(const struct commit *commit, * the cached copy from get_commit_buffer, we need to duplicate it * to avoid munging the cached copy. */ - if (msg == get_cached_commit_buffer(commit)) + if (msg == get_cached_commit_buffer(commit, NULL)) out = xstrdup(msg); else out = (char *)msg; -- cgit v1.2.1