Junio C Hamano <gits...@pobox.com> writes:

> Jeff King <p...@peff.net> 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to