Small refactor to make thing tidier

Also repurposed an unused function and deleted another one.

4 files changed, 46 insertions, 43 deletions

diff --git a/src/diff_generate.c b/src/diff_generate.c
index bc7c26658..f05ae3713 100644
--- a/src/diff_generate.c
+++ b/src/diff_generate.c
@@ -709,18 +709,12 @@ static int maybe_modified_submodule(
 	} else {
 		if (!info->submodule_cache_initialized) {
 			info->submodule_cache_initialized = true;
-			/* Cache the submodule information to avoid having to parse it for every submodule. */
-			if (git_strmap_new(&info->submodule_cache) == 0) {
-				if (git_submodule__map(diff->base.repo, info->submodule_cache) < 0) {
-					/* If the caching failed for whatever reason, bail out and clean up. */
-					git_submodule *sm = NULL;
-					git_strmap_foreach_value(info->submodule_cache, sm, {
-						git_submodule_free(sm);
-					});
-					git_strmap_free(info->submodule_cache);
-					info->submodule_cache = NULL;
-				}
-			}
+			/*
+			 * Try to cache the submodule information to avoid having to parse it for
+			 * every submodule. It is okay if it fails, the cache will still be NULL
+			 * and the submodules will be attempted to be looked up individually.
+			 */
+			git_submodule_cache_init(&info->submodule_cache, diff->base.repo);
 		}
 		submodule_cache = info->submodule_cache;
 	}
@@ -1284,13 +1278,8 @@ cleanup:
 		*out = &diff->base;
 	else
 		git_diff_free(&diff->base);
-	if (info.submodule_cache) {
-		git_submodule *sm = NULL;
-		git_strmap_foreach_value(info.submodule_cache, sm, {
-			git_submodule_free(sm);
-		});
-		git_strmap_free(info.submodule_cache);
-	}
+	if (info.submodule_cache)
+		git_submodule_cache_free(info.submodule_cache);
 
 	return error;
 }
diff --git a/src/repository.c b/src/repository.c
index 3b6bcbe86..3a54f8c44 100644
--- a/src/repository.c
+++ b/src/repository.c
@@ -3055,30 +3055,16 @@ int git_repository_set_ident(git_repository *repo, const char *name, const char
 
 int git_repository_submodule_cache_all(git_repository *repo)
 {
-	int error;
-
 	GIT_ASSERT_ARG(repo);
-
-	if ((error = git_strmap_new(&repo->submodule_cache)))
-		return error;
-
-	error = git_submodule__map(repo, repo->submodule_cache);
-	return error;
+	return git_submodule_cache_init(&repo->submodule_cache, repo);
 }
 
 int git_repository_submodule_cache_clear(git_repository *repo)
 {
-	git_submodule *sm;
-
+	int error = 0;
 	GIT_ASSERT_ARG(repo);
 
-	if (repo->submodule_cache == NULL) {
-		return 0;
-	}
-	git_strmap_foreach_value(repo->submodule_cache, sm, {
-		git_submodule_free(sm);
-	});
-	git_strmap_free(repo->submodule_cache);
-	repo->submodule_cache = 0;
+	error = git_submodule_cache_free(repo->submodule_cache);
+	repo->submodule_cache = NULL;
 	return 0;
 }
diff --git a/src/submodule.c b/src/submodule.c
index 3bbdeed25..50bde2c63 100644
--- a/src/submodule.c
+++ b/src/submodule.c
@@ -249,6 +249,34 @@ out:
 	return error;
 }
 
+int git_submodule_cache_init(git_strmap **out, git_repository *repo)
+{
+	int error = 0;
+	git_strmap *cache = NULL;
+	GIT_ASSERT_ARG(out);
+	GIT_ASSERT_ARG(repo);
+	if ((error = git_strmap_new(&cache)) < 0)
+		return error;
+	if ((error = git_submodule__map(repo, cache)) < 0) {
+		git_submodule_cache_free(cache);
+		return error;
+	}
+	*out = cache;
+	return error;
+}
+
+int git_submodule_cache_free(git_strmap *cache)
+{
+	git_submodule *sm = NULL;
+	if (cache == NULL)
+		return 0;
+	git_strmap_foreach_value(cache, sm, {
+		git_submodule_free(sm);
+	});
+	git_strmap_free(cache);
+	return 0;
+}
+
 int git_submodule_lookup(
 	git_submodule **out, /* NULL if user only wants to test existence */
 	git_repository *repo,
diff --git a/src/submodule.h b/src/submodule.h
index f32f818ea..b01ff68a2 100644
--- a/src/submodule.h
+++ b/src/submodule.h
@@ -101,12 +101,6 @@ struct git_submodule {
 	git_oid wd_oid;
 };
 
-/* Force revalidation of submodule data cache (alloc as needed) */
-extern int git_submodule_cache_refresh(git_repository *repo);
-
-/* Release all submodules */
-extern void git_submodule_cache_free(git_repository *repo);
-
 /* Additional flags on top of public GIT_SUBMODULE_STATUS values */
 enum {
 	GIT_SUBMODULE_STATUS__WD_SCANNED          = (1u << 20),
@@ -122,6 +116,12 @@ enum {
 #define GIT_SUBMODULE_STATUS__CLEAR_INTERNAL(S) \
 	((S) & ~(0xFFFFFFFFu << 20))
 
+/* Initialize an external submodule cache for the provided repo. */
+extern int git_submodule_cache_init(git_strmap **out, git_repository *repo);
+
+/* Release the resources of the submodule cache. */
+extern int git_submodule_cache_free(git_strmap *cache);
+
 /* Submodule lookup with an explicit cache */
 extern int git_submodule__lookup_with_cache(
 	git_submodule **out, git_repository *repo, const char *path, git_strmap *cache);