Add APIs to dup and free git_index_entrys

This adds git_index_entry_dup to make a copy of an existing entry
and git_index_entry_free to release the memory of the copy.  It
also updates the documentation for git_index_get_bypath and
git_index_get_byindex to make it clear that the returned structure
should *not* be modified.

3 files changed, 129 insertions, 12 deletions

diff --git a/include/git2/index.h b/include/git2/index.h
index 42cc4d640..cfb55ae5b 100644
--- a/include/git2/index.h
+++ b/include/git2/index.h
@@ -73,6 +73,8 @@ GIT_BEGIN_DECL
 #define GIT_IDXENTRY_UNPACKED          (1 << 8)
 #define GIT_IDXENTRY_NEW_SKIP_WORKTREE (1 << 9)
 
+#define GIT_IDXENTRY_ALLOCATED         (1 << 10)
+#define GIT_IDXENTRY_ALLOCATED_PATH    (1 << 11)
 
 #define GIT_IDXENTRY_STAGE(E) (((E)->flags & GIT_IDXENTRY_STAGEMASK) >> GIT_IDXENTRY_STAGESHIFT)
 
@@ -284,11 +286,9 @@ GIT_EXTERN(void) git_index_clear(git_index *index);
 /**
  * Get a pointer to one of the entries in the index
  *
- * The values of this entry can be modified (except the path)
- * and the changes will be written back to disk on the next
- * write() call.
- *
- * The entry should not be freed by the caller.
+ * The entry is not modifiable and should not be freed.  If you need a
+ * permanent copy of the entry, use `git_index_entry_dup()` (after which
+ * you will be responsible for calling `git_index_entry_free()`)
  *
  * @param index an existing index object
  * @param n the position of the entry
@@ -300,11 +300,9 @@ GIT_EXTERN(const git_index_entry *) git_index_get_byindex(
 /**
  * Get a pointer to one of the entries in the index
  *
- * The values of this entry can be modified (except the path)
- * and the changes will be written back to disk on the next
- * write() call.
- *
- * The entry should not be freed by the caller.
+ * The entry is not modifiable and should not be freed.  If you need a
+ * permanent copy of the entry, use `git_index_entry_dup()` (after which
+ * you will be responsible for calling `git_index_entry_free()`).
  *
  * @param index an existing index object
  * @param path path to search
@@ -354,8 +352,7 @@ GIT_EXTERN(int) git_index_add(git_index *index, const git_index_entry *source_en
 /**
  * Return the stage number from a git index entry
  *
- * This entry is calculated from the entry's flag
- * attribute like this:
+ * This entry is calculated from the entry's flag attribute like this:
  *
  *	(entry->flags & GIT_IDXENTRY_STAGEMASK) >> GIT_IDXENTRY_STAGESHIFT
  *
@@ -364,6 +361,33 @@ GIT_EXTERN(int) git_index_add(git_index *index, const git_index_entry *source_en
  */
 GIT_EXTERN(int) git_index_entry_stage(const git_index_entry *entry);
 
+/**
+ * Make a copy of an entry that you can keep
+ *
+ * The `git_index_entry` pointers that are returned by the accessor
+ * functions are `const git_index_entry *` so they can't be modified
+ * and their lifetime as objects matches that of the `git_index`.
+ *
+ * This function allows you to make a copy of those entries that can
+ * be modified and which you are responsible for freeing.
+ *
+ * @param entry The entry to be copied
+ * @returns Newly allocated entry (or NULL if allocation failure)
+ */
+GIT_EXTERN(git_index_entry *) git_index_entry_dup(const git_index_entry *entry);
+
+/**
+ * Release the memory for a git_index_entry
+ *
+ * You should only call this on `git_index_entry` objects that you have
+ * obtained through `git_index_entry_dup()`.  It is an error to call this
+ * on the values returned by `git_index_get_byindex()` or
+ * `git_index_get_bypath()`.
+ *
+ * @param entry The entry to be freed
+ */
+GIT_EXTERN(void) git_index_entry_free(git_index_entry *entry);
+
 /**@}*/
 
 /** @name Workdir Index Entry Functions
diff --git a/src/index.c b/src/index.c
index f767dfab7..f68c81769 100644
--- a/src/index.c
+++ b/src/index.c
@@ -550,6 +550,51 @@ const git_index_entry *git_index_get_bypath(
 	return git_index_get_byindex(index, pos);
 }
 
+typedef struct {
+	git_index_entry entry;
+	char pathdata[GIT_FLEX_ARRAY];
+} git_index_entry_with_path;
+
+git_index_entry *git_index_entry_dup(const git_index_entry *src)
+{
+	git_index_entry_with_path *tgt;
+	size_t pathlen;
+
+	if (!src)
+		return NULL;
+
+	pathlen = strlen(src->path);
+
+	tgt = git__calloc(sizeof(git_index_entry_with_path) + pathlen + 1, 1);
+	if (!tgt)
+		return NULL;
+
+	memcpy(&tgt->entry, src, sizeof(tgt->entry));
+	tgt->entry.flags_extended |= GIT_IDXENTRY_ALLOCATED;
+
+	memcpy(tgt->pathdata, src->path, pathlen + 1);
+	tgt->entry.path = tgt->pathdata;
+
+	return (git_index_entry *)tgt;
+}
+
+void git_index_entry_free(git_index_entry *entry)
+{
+	assert(entry);
+
+	if (!(entry->flags_extended & GIT_IDXENTRY_ALLOCATED))
+		return;
+
+	if ((entry->flags_extended & GIT_IDXENTRY_ALLOCATED_PATH) != 0 &&
+		entry->path != ((git_index_entry_with_path *)entry)->pathdata)
+		git__free(entry->path);
+
+	/* ward off accidental double free */
+	entry->flags_extended = (entry->flags_extended & ~GIT_IDXENTRY_ALLOCATED);
+
+	git__free(entry);
+}
+
 void git_index_entry__init_from_stat(git_index_entry *entry, struct stat *st)
 {
 	entry->ctime.seconds = (git_time_t)st->st_ctime;
diff --git a/tests-clar/index/tests.c b/tests-clar/index/tests.c
index 88e374e6e..565f11073 100644
--- a/tests-clar/index/tests.c
+++ b/tests-clar/index/tests.c
@@ -416,3 +416,51 @@ void test_index_tests__remove_directory(void)
 	git_repository_free(repo);
 	cl_fixture_cleanup("index_test");
 }
+
+void test_index_tests__dup_and_free_entries(void)
+{
+	git_repository *repo;
+	git_index *index;
+	const git_index_entry *entry;
+	git_index_entry *dup1, *dup2;
+
+	cl_git_pass(git_repository_open(&repo, cl_fixture("testrepo.git")));
+	cl_git_pass(git_repository_index(&index, repo));
+
+	cl_assert(entry = git_index_get_bypath(index, "COPYING", 0));
+	cl_assert((entry->flags_extended & GIT_IDXENTRY_ALLOCATED) == 0);
+
+	cl_assert(dup1 = git_index_entry_dup(entry));
+	cl_assert((dup1->flags_extended & GIT_IDXENTRY_ALLOCATED) != 0);
+
+	cl_assert_equal_s(entry->path, dup1->path);
+	cl_assert(git_oid_equal(&entry->oid, &dup1->oid));
+
+	cl_assert(entry = git_index_get_byindex(index, 0));
+	cl_assert((entry->flags_extended & GIT_IDXENTRY_ALLOCATED) == 0);
+
+	cl_assert(dup2 = git_index_entry_dup(entry));
+	cl_assert((dup2->flags_extended & GIT_IDXENTRY_ALLOCATED) != 0);
+
+	cl_assert_equal_s(entry->path, dup2->path);
+	cl_assert(git_oid_equal(&entry->oid, &dup2->oid));
+
+	git_index_free(index);
+	git_repository_free(repo);
+
+	/* entry is no longer pointing to valid memory, but dup1 and dup2 are */
+
+	cl_assert_equal_s("COPYING", dup1->path);
+	git_index_entry_free(dup1);
+
+	cl_assert_equal_s(".HEADER", dup2->path);
+
+	/* what would a binding that wanted to set the path do? */
+	dup2->path = git__strdup("newpath");
+	dup2->flags_extended |= GIT_IDXENTRY_ALLOCATED_PATH;
+	git_index_entry_free(dup2);
+
+	/* at this point there should be no memory leaks nor double-frees in
+	 * this function; that will have to be checked by an external tool.
+	 */
+}