Remove entry dup/free functions and fix comments

This removes the functions to duplicate and free copies of a
git_index_entry and updates the comments to explain that you
should just use the public definition of the struct as needed.

include/git2/index.h

@@ -34,6 +34,8 @@ GIT_BEGIN_DECL
 #define GIT_IDXENTRY_VALID     (0x8000)
 #define GIT_IDXENTRY_STAGESHIFT 12
 
+#define GIT_IDXENTRY_STAGE(E) (((E)->flags & GIT_IDXENTRY_STAGEMASK) >> GIT_IDXENTRY_STAGESHIFT)
+
 /**
  * Bitmasks for on-disk fields of `git_index_entry` `flags_extended`
  *
@@ -73,11 +75,6 @@ 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)
-
 /** Time used in a git index entry */
 typedef struct {
 	git_time_t seconds;
@@ -286,9 +283,9 @@ GIT_EXTERN(void) git_index_clear(git_index *index);
 /**
  * Get a pointer to one of the entries in the index
  *
- * 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()`)
+ * The entry is not modifiable and should not be freed.  Because the
+ * `git_index_entry` struct is a publicly defined struct, you should
+ * be able to make your own permanent copy of the data if necessary.
  *
  * @param index an existing index object
  * @param n the position of the entry
@@ -300,9 +297,9 @@ GIT_EXTERN(const git_index_entry *) git_index_get_byindex(
 /**
  * Get a pointer to one of the entries in the index
  *
- * 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()`).
+ * The entry is not modifiable and should not be freed.  Because the
+ * `git_index_entry` struct is a publicly defined struct, you should
+ * be able to make your own permanent copy of the data if necessary.
  *
  * @param index an existing index object
  * @param path path to search
@@ -361,33 +358,6 @@ 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

src/index.c

@@ -550,51 +550,6 @@ 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;

tests-clar/index/tests.c

@@ -416,51 +416,3 @@ 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.
-	 */
-}