Free the log's tip when its walk fails

The normal end also left the walk arguments allocated.

2 files changed, 25 insertions, 14 deletions

diff --git a/source/ui-log.c b/source/ui-log.cindex 2c1d802..3cbbe33 100644--- a/source/ui-log.c+++ b/source/ui-log.c@@ -483,10 +483,7 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern 	// option. 	if (tip && !cgit_valid_rev(tip)) { 		cgit_print_error_page(400, "Bad Request", "Invalid revision");-		if (must_free_tip)-			free((char *)tip);-		strvec_clear(&rev_argv);-		return;+		goto out; 	} 	strvec_push(&rev_argv, tip); @@ -494,10 +491,7 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern 	// which can end the request inside git. 	if (path && path[0] == ':') { 		cgit_print_error_page(400, "Bad Request", "Invalid path");-		if (must_free_tip)-			free((char *)tip);-		strvec_clear(&rev_argv);-		return;+		goto out; 	}  	if (grep && pattern && *pattern) {@@ -518,10 +512,7 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern 			while ((arg = next_token(&pattern))) { 				if (!valid_range_token(arg)) { 					cgit_print_error_page(400, "Bad Request", "Invalid revision");-					if (must_free_tip)-						free((char *)tip);-					strvec_clear(&rev_argv);-					return;+					goto out; 				} 				strvec_push(&rev_argv, arg); 			}@@ -580,8 +571,7 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern 	// not be read from. 	if (prepare_revision_walk(&rev)) { 		cgit_print_error_page(500, "Internal Server Error", "Unable to read the history");-		strvec_clear(&rev_argv);-		return;+		goto out; 	}  	if (pager) {@@ -650,6 +640,8 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern 		html("</tbody>\n"); 	} +out: 	if (must_free_tip) 		free((char *)tip);+	strvec_clear(&rev_argv); }
diff --git a/source/ui-log.c b/source/ui-log.c
index 2c1d802..3cbbe33 100644
--- a/source/ui-log.c
+++ b/source/ui-log.c
@@ -483,10 +483,7 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
 	// option.
 	if (tip && !cgit_valid_rev(tip)) {
 		cgit_print_error_page(400, "Bad Request", "Invalid revision");
-		if (must_free_tip)
-			free((char *)tip);
-		strvec_clear(&rev_argv);
-		return;
+		goto out;
 	}
 	strvec_push(&rev_argv, tip);
 
@@ -494,10 +491,7 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
 	// which can end the request inside git.
 	if (path && path[0] == ':') {
 		cgit_print_error_page(400, "Bad Request", "Invalid path");
-		if (must_free_tip)
-			free((char *)tip);
-		strvec_clear(&rev_argv);
-		return;
+		goto out;
 	}
 
 	if (grep && pattern && *pattern) {
@@ -518,10 +512,7 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
 			while ((arg = next_token(&pattern))) {
 				if (!valid_range_token(arg)) {
 					cgit_print_error_page(400, "Bad Request", "Invalid revision");
-					if (must_free_tip)
-						free((char *)tip);
-					strvec_clear(&rev_argv);
-					return;
+					goto out;
 				}
 				strvec_push(&rev_argv, arg);
 			}
@@ -580,8 +571,7 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
 	// not be read from.
 	if (prepare_revision_walk(&rev)) {
 		cgit_print_error_page(500, "Internal Server Error", "Unable to read the history");
-		strvec_clear(&rev_argv);
-		return;
+		goto out;
 	}
 
 	if (pager) {
@@ -650,6 +640,8 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
 		html("</tbody>\n");
 	}
 
+out:
 	if (must_free_tip)
 		free((char *)tip);
+	strvec_clear(&rev_argv);
 }
diff --git a/tests/t0103-log.sh b/tests/t0103-log.shindex 4306d8c..98b4e0b 100755--- a/tests/t0103-log.sh+++ b/tests/t0103-log.sh@@ -132,4 +132,23 @@ test_expect_success 'a search for brackets matches them literally' ' 	grep "fix \\[brackets\\] in docs" tmp ' +# A range is walked in full before the first row, so a missing parent fails+# the walk while the page can still answer with an error of its own.+test_expect_success 'a walk that cannot start answers one error page' '+	git clone -q repos/foo gap &&+	old=$(git -C gap rev-parse HEAD~4) &&+	parent=$(git -C gap rev-parse HEAD~2) &&+	rm gap/.git/objects/$(echo $parent | cut -c1-2)/$(echo $parent | cut -c3-) &&+	{+		echo "virtual-root=/" &&+		echo "cache-size=0" &&+		echo "repo.url=gap" &&+		echo "repo.path=$PWD/gap/.git"+	} >gaprc &&+	CGIT_CONFIG="$PWD/gaprc" QUERY_STRING="url=gap/log/&qt=range&q=$old..master" cgit >tmp 2>err &&+	test $(grep -c "^Status:" tmp) = 1 &&+	grep "^Status: 500" tmp &&+	grep "Unable to read the history" tmp+'+ test_done
diff --git a/tests/t0103-log.sh b/tests/t0103-log.sh
index 4306d8c..98b4e0b 100755
--- a/tests/t0103-log.sh
+++ b/tests/t0103-log.sh
@@ -132,4 +132,23 @@ test_expect_success 'a search for brackets matches them literally' '
 	grep "fix \\[brackets\\] in docs" tmp
 '
 
+# A range is walked in full before the first row, so a missing parent fails
+# the walk while the page can still answer with an error of its own.
+test_expect_success 'a walk that cannot start answers one error page' '
+	git clone -q repos/foo gap &&
+	old=$(git -C gap rev-parse HEAD~4) &&
+	parent=$(git -C gap rev-parse HEAD~2) &&
+	rm gap/.git/objects/$(echo $parent | cut -c1-2)/$(echo $parent | cut -c3-) &&
+	{
+		echo "virtual-root=/" &&
+		echo "cache-size=0" &&
+		echo "repo.url=gap" &&
+		echo "repo.path=$PWD/gap/.git"
+	} >gaprc &&
+	CGIT_CONFIG="$PWD/gaprc" QUERY_STRING="url=gap/log/&qt=range&q=$old..master" cgit >tmp 2>err &&
+	test $(grep -c "^Status:" tmp) = 1 &&
+	grep "^Status: 500" tmp &&
+	grep "Unable to read the history" tmp
+'
+
 test_done