Commit 54faddd299ccb6187a9747c1d3ee18d33e5edf7a

Russell Belfer 2013-06-12T11:54:11

Fix some diff driver memory leaks

diff --git a/src/diff_driver.c b/src/diff_driver.c
index 9c109e7..5d3274a 100644
--- a/src/diff_driver.c
+++ b/src/diff_driver.c
@@ -163,10 +163,10 @@ static int git_diff_driver_load(
 	memcpy(drv->name, driver_name, namelen);
 
 	if ((error = git_buf_printf(&name, "diff.%s.binary", driver_name)) < 0)
-		goto fail;
+		goto done;
 	if ((error = git_config_get_string(&val, cfg, name.ptr)) < 0) {
 		if (error != GIT_ENOTFOUND)
-			goto fail;
+			goto done;
 		/* diff.<driver>.binary unspecified, so just continue */
 		giterr_clear();
 	} else if (git_config_parse_bool(&bval, val) < 0) {
@@ -174,9 +174,8 @@ static int git_diff_driver_load(
 		giterr_clear();
 	} else if (bval) {
 		/* if diff.<driver>.binary is true, just return the binary driver */
-		git__free(drv);
 		*out = &global_drivers[DIFF_DRIVER_BINARY];
-		return 0;
+		goto done;
 	} else {
 		/* if diff.<driver>.binary is false, force binary checks off */
 		/* but still may have custom function context patterns, etc. */
@@ -191,7 +190,7 @@ static int git_diff_driver_load(
 	if ((error = git_config_get_multivar(
 			cfg, name.ptr, NULL, diff_driver_xfuncname, drv)) < 0) {
 		if (error != GIT_ENOTFOUND)
-			goto fail;
+			goto done;
 		giterr_clear(); /* no diff.<driver>.xfuncname, so just continue */
 	}
 
@@ -200,7 +199,7 @@ static int git_diff_driver_load(
 	if ((error = git_config_get_multivar(
 			cfg, name.ptr, NULL, diff_driver_funcname, drv)) < 0) {
 		if (error != GIT_ENOTFOUND)
-			goto fail;
+			goto done;
 		giterr_clear(); /* no diff.<driver>.funcname, so just continue */
 	}
 
@@ -214,12 +213,12 @@ static int git_diff_driver_load(
 	git_buf_put(&name, "wordregex", strlen("wordregex"));
 	if ((error = git_config_get_string(&val, cfg, name.ptr)) < 0) {
 		if (error != GIT_ENOTFOUND)
-			goto fail;
+			goto done;
 		giterr_clear(); /* no diff.<driver>.wordregex, so just continue */
 	} else if ((error = regcomp(&drv->word_pattern, val, REG_EXTENDED)) != 0) {
 		/* TODO: warning about bad regex instead of failure */
 		error = giterr_set_regex(&drv->word_pattern, error);
-		goto fail;
+		goto done;
 	} else {
 		found_driver = true;
 	}
@@ -228,21 +227,26 @@ static int git_diff_driver_load(
 	 * diff in drv->other_flags
 	 */
 
-	/* if no driver config found, fall back on AUTO driver */
+	/* if no driver config found at all, fall back on AUTO driver */
 	if (!found_driver)
-		goto fail;
+		goto done;
 
 	/* store driver in registry */
 	git_strmap_insert(reg->drivers, drv->name, drv, error);
 	if (error < 0)
-		goto fail;
+		goto done;
 
 	*out = drv;
-	return 0;
 
-fail:
-	git_diff_driver_free(drv);
-	*out = &global_drivers[DIFF_DRIVER_AUTO];
+done:
+	git_buf_free(&name);
+
+	if (!*out)
+		*out = &global_drivers[DIFF_DRIVER_AUTO];
+
+	if (drv && drv != *out)
+		git_diff_driver_free(drv);
+
 	return error;
 }
 
@@ -289,7 +293,7 @@ void git_diff_driver_free(git_diff_driver *driver)
 	if (!driver)
 		return;
 
-	for (i = 0; i > git_array_size(driver->fn_patterns); ++i)
+	for (i = 0; i < git_array_size(driver->fn_patterns); ++i)
 		regfree(git_array_get(driver->fn_patterns, i));
 	git_array_clear(driver->fn_patterns);
 
diff --git a/tests-clar/diff/drivers.c b/tests-clar/diff/drivers.c
index 8f7b1f2..06ab2ff 100644
--- a/tests-clar/diff/drivers.c
+++ b/tests-clar/diff/drivers.c
@@ -4,7 +4,6 @@
 #include "diff_driver.h"
 
 static git_repository *g_repo = NULL;
-static git_config *g_cfg = NULL;
 
 void test_diff_drivers__initialize(void)
 {
@@ -12,15 +11,13 @@ void test_diff_drivers__initialize(void)
 
 void test_diff_drivers__cleanup(void)
 {
-	git_config_free(g_cfg);
-	g_cfg = NULL;
-
 	cl_git_sandbox_cleanup();
 	g_repo = NULL;
 }
 
 void test_diff_drivers__patterns(void)
 {
+	git_config *cfg;
 	const char *one_sha = "19dd32dfb1520a64e5bbaae8dce6ef423dfa2f13";
 	git_tree *one;
 	git_diff_list *diff;
@@ -87,8 +84,9 @@ void test_diff_drivers__patterns(void)
 
 	/* let's define that driver */
 
-	cl_git_pass(git_repository_config(&g_cfg, g_repo));
-	cl_git_pass(git_config_set_bool(g_cfg, "diff.kipling0.binary", 1));
+	cl_git_pass(git_repository_config(&cfg, g_repo));
+	cl_git_pass(git_config_set_bool(cfg, "diff.kipling0.binary", 1));
+	git_config_free(cfg);
 
 	cl_git_pass(git_diff_tree_to_workdir(&diff, g_repo, one, NULL));
 	cl_assert_equal_i(1, (int)git_diff_num_deltas(diff));
@@ -106,9 +104,10 @@ void test_diff_drivers__patterns(void)
 	git_diff_driver_registry_free(g_repo->diff_drivers);
 	g_repo->diff_drivers = NULL;
 
-	cl_git_pass(git_repository_config(&g_cfg, g_repo));
-	cl_git_pass(git_config_set_bool(g_cfg, "diff.kipling0.binary", 0));
-	cl_git_pass(git_config_set_string(g_cfg, "diff.kipling0.xfuncname", "^H"));
+	cl_git_pass(git_repository_config(&cfg, g_repo));
+	cl_git_pass(git_config_set_bool(cfg, "diff.kipling0.binary", 0));
+	cl_git_pass(git_config_set_string(cfg, "diff.kipling0.xfuncname", "^H"));
+	git_config_free(cfg);
 
 	cl_git_pass(git_diff_tree_to_workdir(&diff, g_repo, one, NULL));
 	cl_assert_equal_i(1, (int)git_diff_num_deltas(diff));