get_sha1_oneline: fix lifespan rule of temp_commit_buffer variable

gitster · gitster · commit 28042dbcd60f · 2010-12-12T22:31:45.000-08:00
This is trying to free only what we ourselves read (as opposed to what
we borrowed from commit-&gt;buffer) but do so lazily only to work around
the fact that the code has many irregular exit points, and doing it right
makes it necessary to call free() from many different places in the loop.

Rewrite the structure of the code inside the loop so that the variable
has to live within a single iteration, ever.  This should make the logic
easier to follow as well.

Also we didn't free a temporary commit list we kept to hold the original
set of commits.  Free it.

Noticed-by: Nguyễn Thái Ngọc Duy &lt;pclouds@gmail.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
diff --git a/sha1_name.c b/sha1_name.c
@@ -693,8 +693,7 @@ static int handle_one_ref(const char *path,
 static int get_sha1_oneline(const char *prefix, unsigned char *sha1)
 {
 	struct commit_list *list = NULL, *backup = NULL, *l;
-	int retval = -1;
-	char *temp_commit_buffer = NULL;
+	int found = 0;
 	regex_t regex;
 
 	if (prefix[0] == '!') {
@@ -710,37 +709,40 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1)
 	for (l = list; l; l = l->next)
 		commit_list_insert(l->item, &backup);
 	while (list) {
-		char *p;
+		char *p, *to_free = NULL;
 		struct commit *commit;
 		enum object_type type;
 		unsigned long size;
+		int matches;
 
 		commit = pop_most_recent_commit(&list, ONELINE_SEEN);
 		if (!parse_object(commit->object.sha1))
 			continue;
-		free(temp_commit_buffer);
 		if (commit->buffer)
 			p = commit->buffer;
 		else {
 			p = read_sha1_file(commit->object.sha1, &type, &size);
 			if (!p)
 				continue;
-			temp_commit_buffer = p;
+			to_free = p;
 		}
-		if (!(p = strstr(p, "\n\n")))
-			continue;
-		if (!regexec(&regex, p + 2, 0, NULL, 0)) {
+
+		p = strstr(p, "\n\n");
+		matches = p && !regexec(&regex, p + 2, 0, NULL, 0);
+		free(to_free);
+
+		if (matches) {
 			hashcpy(sha1, commit->object.sha1);
-			retval = 0;
+			found = 1;
 			break;
 		}
 	}
 	regfree(&regex);
-	free(temp_commit_buffer);
 	free_commit_list(list);
 	for (l = backup; l; l = l->next)
 		clear_commit_marks(l->item, ONELINE_SEEN);
-	return retval;
+	free_commit_list(backup);
+	return found ? 0 : -1;
 }
 
 struct grab_nth_branch_switch_cbdata {
