Re: segmentation fault (nullpointer) with git log --submodule -p
Junio C Hamano <gitster@pobox.com> writes:
Show 13 quoted lines
> Jeff King <peff@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;
+}
Junio C Hamano <gitster@pobox.com>