Junio C Hamano <gitster@xxxxxxxxx> writes: > Jeff King <peff@xxxxxxxx> writes: > >> ... (e.g., how should "log" know that a submodule diff might later want >> to see the same entry? Should we optimistically free and then make it >> easier for the later user to reliably ensure the buffer is primed? Or >> should we err on the side of keeping it in place?). > > My knee-jerk reaction is that we should consider that commit->buffer > belongs to the revision traversal machinery. Any other uses bolted > on later can borrow it if buffer still exists (I do not think pretty > code rewrites the buffer contents in place in any way), or they can > ask read_sha1_file() to read it themselves and free when they are > done. I've been toying with an idea along this line. commit.h | 16 ++++++++++++++++ builtin/blame.c | 27 ++++++++------------------- commit.c | 20 ++++++++++++++++++++ 3 files changed, 44 insertions(+), 19 deletions(-) diff --git a/commit.h b/commit.h index c16c8a7..b559535 100644 --- a/commit.h +++ b/commit.h @@ -226,4 +226,20 @@ extern void print_commit_list(struct commit_list *list, const char *format_cur, const char *format_last); +extern int ensure_commit_buffer(struct commit *); +extern void discard_commit_buffer(struct commit *); + +#define with_commit_buffer(commit) \ + do { \ + int had_buffer_ = !!commit->buffer; \ + if (!had_buffer_) \ + ensure_commit_buffer(commit); \ + do + +#define done_with_commit_buffer(commit) \ + while (0); \ + if (!had_buffer_) \ + discard_commit_buffer(commit); \ + } while (0) + #endif /* COMMIT_H */ diff --git a/builtin/blame.c b/builtin/blame.c index b431ba3..8b2e4a5 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -1424,25 +1424,14 @@ static void get_commit_info(struct commit *commit, commit_info_init(ret); - /* - * We've operated without save_commit_buffer, so - * we now need to populate them for output. - */ - if (!commit->buffer) { - enum object_type type; - unsigned long size; - commit->buffer = - read_sha1_file(commit->object.sha1, &type, &size); - if (!commit->buffer) - die("Cannot read commit %s", - sha1_to_hex(commit->object.sha1)); - } - encoding = get_log_output_encoding(); - reencoded = logmsg_reencode(commit, encoding); - message = reencoded ? reencoded : commit->buffer; - get_ac_line(message, "\nauthor ", - &ret->author, &ret->author_mail, - &ret->author_time, &ret->author_tz); + with_commit_buffer(commit) { + encoding = get_log_output_encoding(); + reencoded = logmsg_reencode(commit, encoding); + message = reencoded ? reencoded : commit->buffer; + get_ac_line(message, "\nauthor ", + &ret->author, &ret->author_mail, + &ret->author_time, &ret->author_tz); + } done_with_commit_buffer(commit); if (!detailed) { free(reencoded); diff --git a/commit.c b/commit.c index e8eb0ae..a627eea 100644 --- a/commit.c +++ b/commit.c @@ -1357,3 +1357,23 @@ void print_commit_list(struct commit_list *list, printf(format, sha1_to_hex(list->item->object.sha1)); } } + +int ensure_commit_buffer(struct commit *commit) +{ + enum object_type type; + unsigned long size; + + if (commit->buffer) + return 0; + commit->buffer = read_sha1_file(commit->object.sha1, &type, &size); + if (commit->buffer) + return -1; + else + return 0; +} + +void discard_commit_buffer(struct commit *commit) +{ + free(commit->buffer); + commit->buffer = NULL; +} -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html