Commit 9cfce2735d77f4d8b6005e62349dd97c0c6de5ab

Russell Belfer 2013-12-12T12:11:38

Cleanups, renames, and leak fixes This renames git_vector_free_all to the better git_vector_free_deep and also contains a couple of memory leak fixes based on valgrind checks. The fixes are specifically: failure to free global dir path variables when not compiled with threading on and failure to free filters from the filter registry that had not be initialized fully.

diff --git a/include/git2/sys/filter.h b/include/git2/sys/filter.h
index 94ad3ae..8fe21c9 100644
--- a/include/git2/sys/filter.h
+++ b/include/git2/sys/filter.h
@@ -149,6 +149,7 @@ typedef int (*git_filter_init_fn)(git_filter *self);
  * Specified as `filter.shutdown`, this is an optional callback invoked
  * when the filter is unregistered or when libgit2 is shutting down.  It
  * will be called once at most and should release resources as needed.
+ * This may be called even if the `initialize` callback was not made.
  *
  * Typically this function will free the `git_filter` object itself.
  */
diff --git a/src/blame.c b/src/blame.c
index f10ed40..a135741 100644
--- a/src/blame.c
+++ b/src/blame.c
@@ -139,7 +139,7 @@ void git_blame_free(git_blame *blame)
 		free_hunk(hunk);
 	git_vector_free(&blame->hunks);
 
-	git_vector_free_all(&blame->paths);
+	git_vector_free_deep(&blame->paths);
 
 	git_array_clear(blame->line_index);
 
diff --git a/src/checkout.c b/src/checkout.c
index f7dd052..0f30d16 100644
--- a/src/checkout.c
+++ b/src/checkout.c
@@ -1765,7 +1765,7 @@ static void checkout_data_clear(checkout_data *data)
 	git_vector_free(&data->removes);
 	git_pool_clear(&data->pool);
 
-	git_vector_free_all(&data->conflicts);
+	git_vector_free_deep(&data->conflicts);
 
 	git__free(data->pfx);
 	data->pfx = NULL;
diff --git a/src/diff.c b/src/diff.c
index 83adc2a..7f2e58c 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -476,7 +476,7 @@ static int diff_list_apply_options(
 
 static void diff_list_free(git_diff *diff)
 {
-	git_vector_free_all(&diff->deltas);
+	git_vector_free_deep(&diff->deltas);
 
 	git_pathspec__vfree(&diff->pathspec);
 	git_pool_clear(&diff->pool);
diff --git a/src/diff_tform.c b/src/diff_tform.c
index da4bdb3..263a64d 100644
--- a/src/diff_tform.c
+++ b/src/diff_tform.c
@@ -209,7 +209,7 @@ int git_diff_merge(git_diff *onto, const git_diff *from)
 			git_pool_strdup_safe(&onto->pool, onto->opts.new_prefix);
 	}
 
-	git_vector_free_all(&onto_new);
+	git_vector_free_deep(&onto_new);
 	git_pool_clear(&onto_pool);
 
 	return error;
@@ -440,7 +440,7 @@ static int apply_splits_and_deletes(
 	return 0;
 
 on_error:
-	git_vector_free_all(&onto);
+	git_vector_free_deep(&onto);
 
 	return -1;
 }
diff --git a/src/fileops.c b/src/fileops.c
index 98dcd32..a60689f 100644
--- a/src/fileops.c
+++ b/src/fileops.c
@@ -615,6 +615,8 @@ static git_futils_dirs_guess_cb git_futils__dir_guess[GIT_FUTILS_DIR__MAX] = {
 	git_futils_guess_template_dirs,
 };
 
+static int git_futils__dirs_shutdown_set = 0;
+
 void git_futils_dirs_global_shutdown(void)
 {
 	int i;
@@ -631,8 +633,6 @@ int git_futils_dirs_global_init(void)
 	for (i = 0; !error && i < GIT_FUTILS_DIR__MAX; i++)
 		error = git_futils_dirs_get(&path, i);
 
-	git__on_shutdown(git_futils_dirs_global_shutdown);
-
 	return error;
 }
 
@@ -652,9 +652,16 @@ int git_futils_dirs_get(const git_buf **out, git_futils_dir_t which)
 
 	GITERR_CHECK_ERROR(git_futils_check_selector(which));
 
-	if (!git_buf_len(&git_futils__dirs[which]))
+	if (!git_buf_len(&git_futils__dirs[which])) {
+		/* prepare shutdown if we're going to need it */
+		if (!git_futils__dirs_shutdown_set) {
+			git__on_shutdown(git_futils_dirs_global_shutdown);
+			git_futils__dirs_shutdown_set = 1;
+		}
+
 		GITERR_CHECK_ERROR(
 			git_futils__dir_guess[which](&git_futils__dirs[which]));
+	}
 
 	*out = &git_futils__dirs[which];
 	return 0;
diff --git a/src/filter.c b/src/filter.c
index 9f866fe..ff81eb1 100644
--- a/src/filter.c
+++ b/src/filter.c
@@ -69,7 +69,7 @@ static void filter_registry_shutdown(void)
 		return;
 
 	git_vector_foreach(&reg->filters, pos, fdef) {
-		if (fdef->initialized && fdef->filter && fdef->filter->shutdown) {
+		if (fdef->filter && fdef->filter->shutdown) {
 			fdef->filter->shutdown(fdef->filter);
 			fdef->initialized = false;
 		}
diff --git a/src/indexer.c b/src/indexer.c
index 718c698..6132571 100644
--- a/src/indexer.c
+++ b/src/indexer.c
@@ -1010,7 +1010,7 @@ void git_indexer_free(git_indexer *idx)
 	if (idx == NULL)
 		return;
 
-	git_vector_free_all(&idx->objects);
+	git_vector_free_deep(&idx->objects);
 
 	if (idx->pack) {
 		struct git_pack_entry *pentry;
@@ -1020,7 +1020,7 @@ void git_indexer_free(git_indexer *idx)
 		git_oidmap_free(idx->pack->idx_cache);
 	}
 
-	git_vector_free_all(&idx->deltas);
+	git_vector_free_deep(&idx->deltas);
 	git_packfile_free(idx->pack);
 	git_filebuf_cleanup(&idx->pack_file);
 	git__free(idx);
diff --git a/src/iterator.c b/src/iterator.c
index 118bbb8..0e7d0db 100644
--- a/src/iterator.c
+++ b/src/iterator.c
@@ -920,7 +920,7 @@ static fs_iterator_frame *fs_iterator__alloc_frame(fs_iterator *fi)
 
 static void fs_iterator__free_frame(fs_iterator_frame *ff)
 {
-	git_vector_free_all(&ff->entries);
+	git_vector_free_deep(&ff->entries);
 	git__free(ff);
 }
 
diff --git a/src/merge.c b/src/merge.c
index 5640be5..d6db192 100644
--- a/src/merge.c
+++ b/src/merge.c
@@ -2383,7 +2383,7 @@ done:
         git_index_set_caps(index_repo, index_repo_caps);
 
 	git_index_free(index_repo);
-	git_vector_free_all(&paths);
+	git_vector_free_deep(&paths);
 
 	return error;
 }
diff --git a/src/pathspec.c b/src/pathspec.c
index f16e19f..bad8dac 100644
--- a/src/pathspec.c
+++ b/src/pathspec.c
@@ -102,7 +102,7 @@ int git_pathspec__vinit(
 /* free data from the pathspec vector */
 void git_pathspec__vfree(git_vector *vspec)
 {
-	git_vector_free_all(vspec);
+	git_vector_free_deep(vspec);
 }
 
 struct pathspec_match_context {
diff --git a/src/push.c b/src/push.c
index adba880..dd77864 100644
--- a/src/push.c
+++ b/src/push.c
@@ -541,7 +541,7 @@ static int queue_objects(git_push *push)
 	error = 0;
 
 on_error:
-	git_vector_free_all(&commits);
+	git_vector_free_deep(&commits);
 	return error;
 }
 
diff --git a/src/remote.c b/src/remote.c
index 689de23..294a870 100644
--- a/src/remote.c
+++ b/src/remote.c
@@ -1138,7 +1138,7 @@ int git_remote_list(git_strarray *remotes_list, git_repository *repo)
 		cfg, "^remote\\..*\\.(push)?url$", remote_list_cb, &list);
 
 	if (error < 0) {
-		git_vector_free_all(&list);
+		git_vector_free_deep(&list);
 		return error;
 	}
 
@@ -1617,7 +1617,7 @@ static int copy_refspecs(git_strarray *array, git_remote *remote, unsigned int p
 	return 0;
 
 on_error:
-	git_vector_free_all(&refspecs);
+	git_vector_free_deep(&refspecs);
 
 	return -1;
 }
diff --git a/src/status.c b/src/status.c
index 9bde8fb..7a1472d 100644
--- a/src/status.c
+++ b/src/status.c
@@ -367,7 +367,7 @@ void git_status_list_free(git_status_list *status)
 	git_diff_free(status->head2idx);
 	git_diff_free(status->idx2wd);
 
-	git_vector_free_all(&status->paired);
+	git_vector_free_deep(&status->paired);
 
 	git__memzero(status, sizeof(*status));
 	git__free(status);
diff --git a/src/transports/http.c b/src/transports/http.c
index 0e1bbf6..c6aaeb9 100644
--- a/src/transports/http.c
+++ b/src/transports/http.c
@@ -404,7 +404,7 @@ static void clear_parser_state(http_subtransport *t)
 	git__free(t->location);
 	t->location = NULL;
 
-	git_vector_free_all(&t->www_authenticate);
+	git_vector_free_deep(&t->www_authenticate);
 }
 
 static int write_chunk(gitno_socket *socket, const char *buffer, size_t len)
diff --git a/src/vector.c b/src/vector.c
index b1ea896..050e032 100644
--- a/src/vector.c
+++ b/src/vector.c
@@ -77,7 +77,7 @@ void git_vector_free(git_vector *v)
 	v->_alloc_size = 0;
 }
 
-void git_vector_free_all(git_vector *v)
+void git_vector_free_deep(git_vector *v)
 {
 	size_t i;
 
diff --git a/src/vector.h b/src/vector.h
index defe224..d318463 100644
--- a/src/vector.h
+++ b/src/vector.h
@@ -23,7 +23,7 @@ typedef struct git_vector {
 
 int git_vector_init(git_vector *v, size_t initial_size, git_vector_cmp cmp);
 void git_vector_free(git_vector *v);
-void git_vector_free_all(git_vector *v); /* free each entry and self */
+void git_vector_free_deep(git_vector *v); /* free each entry and self */
 void git_vector_clear(git_vector *v);
 int git_vector_dup(git_vector *v, const git_vector *src, git_vector_cmp cmp);
 void git_vector_swap(git_vector *a, git_vector *b);