Commit 385449b1df593440abcf6636c81eb5ea19b1d1a1

Carlos Martín Nieto 2015-03-04T01:23:20

note: use a git_buf to return the default namespace The caller has otherwise no way to know how long the string will be allocated or ability to free it. This fixes #2944.

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 34553aa..112b8e9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -86,6 +86,9 @@ v0.22 + 1
   non-snapshot configuration, as there can be no guarantee that the
   returned pointer is valid.
 
+* `git_note_default_ref()` now uses a `git_buf` to return the string,
+  as the string is otherwise not guaranteed to stay allocated.
+
 v0.22
 ------
 
diff --git a/include/git2/notes.h b/include/git2/notes.h
index bb7c02d..3a626ca 100644
--- a/include/git2/notes.h
+++ b/include/git2/notes.h
@@ -183,12 +183,12 @@ GIT_EXTERN(void) git_note_free(git_note *note);
 /**
  * Get the default notes reference for a repository
  *
- * @param out Pointer to the default notes reference
+ * @param out buffer in which to store the name of the default notes reference
  * @param repo The Git repository
  *
  * @return 0 or an error code
  */
-GIT_EXTERN(int) git_note_default_ref(const char **out, git_repository *repo);
+GIT_EXTERN(int) git_note_default_ref(git_buf *out, git_repository *repo);
 
 /**
  * Loop over all the notes within a specified namespace
diff --git a/src/notes.c b/src/notes.c
index 1850e81..ef4b41b 100644
--- a/src/notes.c
+++ b/src/notes.c
@@ -388,7 +388,7 @@ cleanup:
 	return error;
 }
 
-static int note_get_default_ref(const char **out, git_repository *repo)
+static int note_get_default_ref(char **out, git_repository *repo)
 {
 	git_config *cfg;
 	int ret = git_repository_config__weakptr(&cfg, repo);
@@ -399,27 +399,31 @@ static int note_get_default_ref(const char **out, git_repository *repo)
 	return ret;
 }
 
-static int normalize_namespace(const char **notes_ref, git_repository *repo)
+static int normalize_namespace(char **out, git_repository *repo, const char *notes_ref)
 {
-	if (*notes_ref)
+	if (notes_ref) {
+		*out = git__strdup(notes_ref);
+		GITERR_CHECK_ALLOC(*out);
 		return 0;
+	}
 
-	return note_get_default_ref(notes_ref, repo);
+	return note_get_default_ref(out, repo);
 }
 
 static int retrieve_note_tree_and_commit(
 	git_tree **tree_out,
 	git_commit **commit_out,
+	char **notes_ref_out,
 	git_repository *repo,
-	const char **notes_ref)
+	const char *notes_ref)
 {
 	int error;
 	git_oid oid;
 
-	if ((error = normalize_namespace(notes_ref, repo)) < 0)
+	if ((error = normalize_namespace(notes_ref_out, repo, notes_ref)) < 0)
 		return error;
 
-	if ((error = git_reference_name_to_id(&oid, repo, *notes_ref)) < 0)
+	if ((error = git_reference_name_to_id(&oid, repo, *notes_ref_out)) < 0)
 		return error;
 
 	if (git_commit_lookup(commit_out, repo, &oid) < 0)
@@ -432,10 +436,10 @@ static int retrieve_note_tree_and_commit(
 }
 
 int git_note_read(git_note **out, git_repository *repo,
-		  const char *notes_ref, const git_oid *oid)
+		  const char *notes_ref_in, const git_oid *oid)
 {
 	int error;
-	char *target = NULL;
+	char *target = NULL, *notes_ref = NULL;
 	git_tree *tree = NULL;
 	git_commit *commit = NULL;
 
@@ -443,9 +447,10 @@ int git_note_read(git_note **out, git_repository *repo,
 	GITERR_CHECK_ALLOC(target);
 
 	if (!(error = retrieve_note_tree_and_commit(
-			&tree, &commit, repo, &notes_ref)))
+		      &tree, &commit, &notes_ref, repo, notes_ref_in)))
 		error = note_lookup(out, repo, commit, tree, target);
 
+	git__free(notes_ref);
 	git__free(target);
 	git_tree_free(tree);
 	git_commit_free(commit);
@@ -455,7 +460,7 @@ int git_note_read(git_note **out, git_repository *repo,
 int git_note_create(
 	git_oid *out,
 	git_repository *repo,
-	const char *notes_ref,
+	const char *notes_ref_in,
 	const git_signature *author,
 	const git_signature *committer,
 	const git_oid *oid,
@@ -463,14 +468,14 @@ int git_note_create(
 	int allow_note_overwrite)
 {
 	int error;
-	char *target = NULL;
+	char *target = NULL, *notes_ref = NULL;
 	git_commit *commit = NULL;
 	git_tree *tree = NULL;
 
 	target = git_oid_allocfmt(oid);
 	GITERR_CHECK_ALLOC(target);
 
-	error = retrieve_note_tree_and_commit(&tree, &commit, repo, &notes_ref);
+	error = retrieve_note_tree_and_commit(&tree, &commit, &notes_ref, repo, notes_ref_in);
 
 	if (error < 0 && error != GIT_ENOTFOUND)
 		goto cleanup;
@@ -479,18 +484,19 @@ int git_note_create(
 			note, tree, target, &commit, allow_note_overwrite);
 
 cleanup:
+	git__free(notes_ref);
 	git__free(target);
 	git_commit_free(commit);
 	git_tree_free(tree);
 	return error;
 }
 
-int git_note_remove(git_repository *repo, const char *notes_ref,
+int git_note_remove(git_repository *repo, const char *notes_ref_in,
 		const git_signature *author, const git_signature *committer,
 		const git_oid *oid)
 {
 	int error;
-	char *target = NULL;
+	char *target = NULL, *notes_ref;
 	git_commit *commit = NULL;
 	git_tree *tree = NULL;
 
@@ -498,20 +504,31 @@ int git_note_remove(git_repository *repo, const char *notes_ref,
 	GITERR_CHECK_ALLOC(target);
 
 	if (!(error = retrieve_note_tree_and_commit(
-			&tree, &commit, repo, &notes_ref)))
+		      &tree, &commit, &notes_ref, repo, notes_ref_in)))
 		error = note_remove(
 			repo, author, committer, notes_ref, tree, target, &commit);
 
+	git__free(notes_ref);
 	git__free(target);
 	git_commit_free(commit);
 	git_tree_free(tree);
 	return error;
 }
 
-int git_note_default_ref(const char **out, git_repository *repo)
+int git_note_default_ref(git_buf *out, git_repository *repo)
 {
-	assert(repo);
-	return note_get_default_ref(out, repo);
+	char *default_ref;
+	int error;
+
+	assert(out && repo);
+
+	git_buf_sanitize(out);
+
+	if ((error = note_get_default_ref(&default_ref, repo)) < 0)
+		return error;
+
+	git_buf_attach(out, default_ref, strlen(default_ref));
+	return 0;
 }
 
 const git_signature *git_note_committer(const git_note *note)
@@ -635,13 +652,14 @@ void git_note_iterator_free(git_note_iterator *it)
 int git_note_iterator_new(
 	git_note_iterator **it,
 	git_repository *repo,
-	const char *notes_ref)
+	const char *notes_ref_in)
 {
 	int error;
 	git_commit *commit = NULL;
 	git_tree *tree = NULL;
+	char *notes_ref;
 
-	error = retrieve_note_tree_and_commit(&tree, &commit, repo, &notes_ref);
+	error = retrieve_note_tree_and_commit(&tree, &commit, &notes_ref, repo, notes_ref_in);
 	if (error < 0)
 		goto cleanup;
 
@@ -649,6 +667,7 @@ int git_note_iterator_new(
 		git_iterator_free(*it);
 
 cleanup:
+	git__free(notes_ref);
 	git_tree_free(tree);
 	git_commit_free(commit);
 
diff --git a/tests/notes/notes.c b/tests/notes/notes.c
index d3b915e..a91bf5b 100644
--- a/tests/notes/notes.c
+++ b/tests/notes/notes.c
@@ -1,5 +1,7 @@
 #include "clar_libgit2.h"
 
+#include "buffer.h"
+
 static git_repository *_repo;
 static git_signature *_sig;
 
@@ -150,7 +152,7 @@ void test_notes_notes__inserting_a_note_without_passing_a_namespace_uses_the_def
 {
 	git_oid note_oid, target_oid;
 	git_note *note, *default_namespace_note;
-	const char *default_ref;
+	git_buf default_ref = GIT_BUF_INIT;
 
 	cl_git_pass(git_oid_fromstr(&target_oid, "08b041783f40edfe12bb406c9c9a8a040177c125"));
 	cl_git_pass(git_note_default_ref(&default_ref, _repo));
@@ -158,11 +160,12 @@ void test_notes_notes__inserting_a_note_without_passing_a_namespace_uses_the_def
 	create_note(&note_oid, NULL, "08b041783f40edfe12bb406c9c9a8a040177c125", "hello world\n");
 
 	cl_git_pass(git_note_read(&note, _repo, NULL, &target_oid));
-	cl_git_pass(git_note_read(&default_namespace_note, _repo, default_ref, &target_oid));
+	cl_git_pass(git_note_read(&default_namespace_note, _repo, git_buf_cstr(&default_ref), &target_oid));
 
 	assert_note_equal(note, "hello world\n", &note_oid);
 	assert_note_equal(default_namespace_note, "hello world\n", &note_oid);
 
+	git_buf_free(&default_ref);
 	git_note_free(note);
 	git_note_free(default_namespace_note);
 }
diff --git a/tests/notes/notesref.c b/tests/notes/notesref.c
index 11391e4..4159ddc 100644
--- a/tests/notes/notesref.c
+++ b/tests/notes/notesref.c
@@ -1,6 +1,7 @@
 #include "clar_libgit2.h"
 
 #include "notes.h"
+#include "buffer.h"
 
 static git_repository *_repo;
 static git_note *_note;
@@ -33,7 +34,7 @@ void test_notes_notesref__cleanup(void)
 void test_notes_notesref__config_corenotesref(void)
 {
 	git_oid oid, note_oid;
-	const char *default_ref;
+	git_buf default_ref = GIT_BUF_INIT;
 
 	cl_git_pass(git_signature_now(&_sig, "alice", "alice@example.com"));
 	cl_git_pass(git_oid_fromstr(&oid, "8496071c1b46c854b31185ea97743be6a8774479"));
@@ -55,10 +56,13 @@ void test_notes_notesref__config_corenotesref(void)
 	cl_assert_equal_oid(git_note_id(_note), &note_oid);
 
 	cl_git_pass(git_note_default_ref(&default_ref, _repo));
-	cl_assert_equal_s("refs/notes/mydefaultnotesref", default_ref);
+	cl_assert_equal_s("refs/notes/mydefaultnotesref", default_ref.ptr);
+	git_buf_clear(&default_ref);
 
 	cl_git_pass(git_config_delete_entry(_cfg, "core.notesRef"));
 
 	cl_git_pass(git_note_default_ref(&default_ref, _repo));
-	cl_assert_equal_s(GIT_NOTES_DEFAULT_REF, default_ref);
+	cl_assert_equal_s(GIT_NOTES_DEFAULT_REF, default_ref.ptr);
+
+	git_buf_free(&default_ref);
 }