Re: segmentation fault (nullpointer) with git log --submodule -p

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]