[PATCH 2/2] submodule--helper: free URL when repository setup fails

[Jeff King <peff@peff.net>]

If repo setup fails, we'll return an error without freeing the allocated
url string, leaking the memory. The test suite does trigger this error,
but never with the leak. We only allocate a url if submodule_from_path()
returned something, but our tests use other situations, like totally
nonexistent submodules.

We can cover this case by asking about a submodule that exists but which
has not been initialized. The new test fails with SANITIZE=leak.

The smallest fix would just be a call to free(url), but I think it's a
little nicer to set up a dedicated out-path for cleanup here. The
previous commit made it safe to call repo_clear() even if
repo_submodule_init() fails.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/submodule--helper.c             | 10 +++++++---
 t/t7426-submodule-get-default-remote.sh | 17 +++++++++++++++++
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index e7cd3225fa..469e3dbcc9 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -80,6 +80,7 @@ static int get_default_remote_submodule(const char *module_path, char **default_
 	struct repository subrepo;
 	const char *remote_name = NULL;
 	char *url = NULL;
+	int ret = 0;
 
 	sub = submodule_from_path(the_repository, null_oid(the_hash_algo), module_path);
 	if (sub && sub->url) {
@@ -96,9 +97,11 @@ static int get_default_remote_submodule(const char *module_path, char **default_
 	}
 
 	if (repo_submodule_init(&subrepo, the_repository, module_path,
-				null_oid(the_hash_algo)) < 0)
-		return die_message(_("could not get a repository handle for submodule '%s'"),
+				null_oid(the_hash_algo)) < 0) {
+		ret = die_message(_("could not get a repository handle for submodule '%s'"),
 				   module_path);
+		goto out;
+	}
 
 	/* Look up by URL first */
 	if (url)
@@ -108,10 +111,11 @@ static int get_default_remote_submodule(const char *module_path, char **default_
 
 	*default_remote = xstrdup(remote_name);
 
+out:
 	repo_clear(&subrepo);
 	free(url);
 
-	return 0;
+	return ret;
 }
 
 static int module_get_default_remote(int argc, const char **argv, const char *prefix,
diff --git a/t/t7426-submodule-get-default-remote.sh b/t/t7426-submodule-get-default-remote.sh
index b842af9a2d..0379c9f044 100755
--- a/t/t7426-submodule-get-default-remote.sh
+++ b/t/t7426-submodule-get-default-remote.sh
@@ -60,6 +60,23 @@ test_expect_success 'get-default-remote fails with non-submodule path' '
 	)
 '
 
+test_expect_success 'get-default-remote fails with uninitialized submodule' '
+	test_when_finished "
+		git -C super config -f .gitmodules --remove-section submodule.uninitialized &&
+		git -C super update-index --force-remove uninitialized
+	" &&
+	(
+		cd super &&
+		git config -f .gitmodules submodule.uninitialized.path uninitialized &&
+		git config -f .gitmodules submodule.uninitialized.url ../sub &&
+		head=$(git -C ../sub rev-parse HEAD) &&
+		git update-index --add --cacheinfo 160000,$head,uninitialized &&
+		test_must_fail git submodule--helper get-default-remote \
+			uninitialized 2>err &&
+		test_grep "could not get a repository handle" err
+	)
+'
+
 test_expect_success 'get-default-remote fails without path argument' '
 	(
 		cd super &&
-- 
2.55.0.1067.gf7fc94a55c