treebuilder: don't keep removed entries around If the user wants to keep a copy for themselves, they should make a copy. It adds unnecessary complexity to make sure the returned entries are valid until the builder is cleared.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158
diff --git a/include/git2/tree.h b/include/git2/tree.h
index 8f1d8a0..42b6819 100644
--- a/include/git2/tree.h
+++ b/include/git2/tree.h
@@ -301,8 +301,10 @@ GIT_EXTERN(const git_tree_entry *) git_treebuilder_get(
* If an entry named `filename` already exists, its attributes
* will be updated with the given ones.
*
- * The optional pointer `out` can be used to retrieve a pointer to
- * the newly created/updated entry. Pass NULL if you do not need it.
+ * The optional pointer `out` can be used to retrieve a pointer to the
+ * newly created/updated entry. Pass NULL if you do not need it. The
+ * pointer may not be valid past the next operation in this
+ * builder. Duplicate the entry if you want to keep it.
*
* No attempt is being made to ensure that the provided oid points
* to an existing git object in the object database, nor that the
@@ -354,7 +356,7 @@ typedef int (*git_treebuilder_filter_cb)(
* @param filter Callback to filter entries
* @param payload Extra data to pass to filter callback
*/
-GIT_EXTERN(int) git_treebuilder_filter(
+GIT_EXTERN(void) git_treebuilder_filter(
git_treebuilder *bld,
git_treebuilder_filter_cb filter,
void *payload);
diff --git a/src/tree.c b/src/tree.c
index 7d28c86..e7357dd 100644
--- a/src/tree.c
+++ b/src/tree.c
@@ -625,9 +625,6 @@ int git_treebuilder_create(git_treebuilder **builder_p, const git_tree *source)
return -1;
}
- if (git_vector_init(&bld->removed, 0, NULL) < 0)
- goto on_error;
-
if (source != NULL) {
git_tree_entry *entry_src;
@@ -657,7 +654,7 @@ int git_treebuilder_insert(
{
git_tree_entry *entry;
int error;
- git_strmap_iter iter;
+ git_strmap_iter pos;
assert(bld && id && filename);
@@ -667,22 +664,20 @@ int git_treebuilder_insert(
if (!valid_entry_name(filename))
return tree_error("Failed to insert entry. Invalid name for a tree entry", filename);
- entry = alloc_entry(filename);
- GITERR_CHECK_ALLOC(entry);
-
- iter = git_strmap_lookup_index(bld->map, entry->filename);
- if (git_strmap_valid_index(bld->map, iter)) {
- git_tree_entry *old_val = git_strmap_value_at(bld->map, iter);
- if (git_vector_insert(&bld->removed, old_val) < 0)
- return -1;
+ pos = git_strmap_lookup_index(bld->map, filename);
+ if (git_strmap_valid_index(bld->map, pos)) {
+ entry = git_strmap_value_at(bld->map, pos);
+ } else {
+ entry = alloc_entry(filename);
+ GITERR_CHECK_ALLOC(entry);
- git_strmap_delete_at(bld->map, iter);
- }
+ git_strmap_insert(bld->map, entry->filename, entry, error);
- git_strmap_insert(bld->map, entry->filename, entry, error);
- if (error < 0) {
- giterr_set(GITERR_TREE, "failed to insert %s", filename);
- return -1;
+ if (error < 0) {
+ git_tree_entry_free(entry);
+ giterr_set(GITERR_TREE, "failed to insert %s", filename);
+ return -1;
+ }
}
bld->entrycount++;
@@ -721,10 +716,8 @@ int git_treebuilder_remove(git_treebuilder *bld, const char *filename)
if (entry == NULL)
return tree_error("Failed to remove entry. File isn't in the tree", filename);
- if (git_vector_insert(&bld->removed, entry) < 0)
- return -1;
-
git_strmap_delete(bld->map, filename);
+ git_tree_entry_free(entry);
bld->entrycount--;
return 0;
@@ -775,7 +768,7 @@ int git_treebuilder_write(git_oid *oid, git_repository *repo, git_treebuilder *b
return error;
}
-int git_treebuilder_filter(
+void git_treebuilder_filter(
git_treebuilder *bld,
git_treebuilder_filter_cb filter,
void *payload)
@@ -787,30 +780,21 @@ int git_treebuilder_filter(
git_strmap_foreach(bld->map, filename, entry, {
if (filter(entry, payload)) {
- if (git_vector_insert(&bld->removed, entry) < 0)
- return -1;
-
git_strmap_delete(bld->map, filename);
bld->entrycount--;
+ git_tree_entry_free(entry);
}
});
-
- return 0;
}
void git_treebuilder_clear(git_treebuilder *bld)
{
- size_t i;
git_tree_entry *e;
assert(bld);
- git_vector_foreach(&bld->removed, i, e)
- git_tree_entry_free(e);
-
git_strmap_foreach_value(bld->map, e, git_tree_entry_free(e));
-
- git_vector_clear(&bld->removed);
+ git_strmap_clear(bld->map);
bld->entrycount = 0;
}
@@ -820,7 +804,6 @@ void git_treebuilder_free(git_treebuilder *bld)
return;
git_treebuilder_clear(bld);
- git_vector_free(&bld->removed);
git_strmap_free(bld->map);
git__free(bld);
}
diff --git a/src/tree.h b/src/tree.h
index 38daf21..81508a6 100644
--- a/src/tree.h
+++ b/src/tree.h
@@ -26,7 +26,6 @@ struct git_tree {
};
struct git_treebuilder {
- git_vector removed;
size_t entrycount; /* vector may contain "removed" entries */
git_strmap *map;
};