Commit ab0421612f045fa2999296facb5fdab373bf7196

Edward Thomson 2022-01-18T08:12:18

tree: move git_oid into tree entry A tree entry previously pointed directly into the object id within the tree object itself; this is useful to avoid any unnecessary memory copy (and an unnecessary use of 40 bytes per tree entry) but difficult if we change the underlying `git_oid` object to not simply be a raw object id but have additional structure. This commit moves the `git_oid` directly into the tree entry; this simplifies the tree entry creation from user data. We now copy the `git_oid` into place when parsing.

diff --git a/src/libgit2/indexer.c b/src/libgit2/indexer.c
index f9a32e7..a7651bc 100644
--- a/src/libgit2/indexer.c
+++ b/src/libgit2/indexer.c
@@ -385,7 +385,7 @@ static int check_object_connectivity(git_indexer *idx, const git_rawobj *obj)
 			size_t i;
 
 			git_array_foreach(tree->entries, i, entry)
-				if (add_expected_oid(idx, entry->oid) < 0)
+				if (add_expected_oid(idx, &entry->oid) < 0)
 					goto out;
 
 			break;
diff --git a/src/libgit2/iterator.c b/src/libgit2/iterator.c
index 15bb63d..bc6f766 100644
--- a/src/libgit2/iterator.c
+++ b/src/libgit2/iterator.c
@@ -613,7 +613,7 @@ GIT_INLINE(int) tree_iterator_frame_push_neighbors(
 			break;
 
 		if ((error = git_tree_lookup(&tree,
-			iter->base.repo, entry->tree_entry->oid)) < 0)
+			iter->base.repo, &entry->tree_entry->oid)) < 0)
 			break;
 
 		if (git_vector_insert(&parent_frame->similar_trees, tree) < 0)
@@ -659,7 +659,7 @@ GIT_INLINE(int) tree_iterator_frame_push(
 	int error;
 
 	if ((error = git_tree_lookup(&tree,
-			iter->base.repo, entry->tree_entry->oid)) < 0 ||
+			iter->base.repo, &entry->tree_entry->oid)) < 0 ||
 		(error = tree_iterator_frame_init(iter, tree, entry)) < 0)
 		goto done;
 
@@ -740,7 +740,7 @@ static void tree_iterator_set_current(
 
 	iter->entry.mode = tree_entry->attr;
 	iter->entry.path = iter->entry_path.ptr;
-	git_oid_cpy(&iter->entry.id, tree_entry->oid);
+	git_oid_cpy(&iter->entry.id, &tree_entry->oid);
 }
 
 static int tree_iterator_advance(const git_index_entry **out, git_iterator *i)
diff --git a/src/libgit2/tree.c b/src/libgit2/tree.c
index a1545dc..a5371fd 100644
--- a/src/libgit2/tree.c
+++ b/src/libgit2/tree.c
@@ -82,6 +82,7 @@ int git_tree_entry_cmp(const git_tree_entry *e1, const git_tree_entry *e2)
 static git_tree_entry *alloc_entry(const char *filename, size_t filename_len, const git_oid *id)
 {
 	git_tree_entry *entry = NULL;
+	char *filename_ptr;
 	size_t tree_len;
 
 	TREE_ENTRY_CHECK_NAMELEN(filename_len);
@@ -95,21 +96,13 @@ static git_tree_entry *alloc_entry(const char *filename, size_t filename_len, co
 	if (!entry)
 		return NULL;
 
-	{
-		char *filename_ptr;
-		void *id_ptr;
-
-		filename_ptr = ((char *) entry) + sizeof(git_tree_entry);
-		memcpy(filename_ptr, filename, filename_len);
-		entry->filename = filename_ptr;
-
-		id_ptr = filename_ptr + filename_len + 1;
-		git_oid_cpy(id_ptr, id);
-		entry->oid = id_ptr;
-	}
-
+	filename_ptr = ((char *) entry) + sizeof(git_tree_entry);
+	memcpy(filename_ptr, filename, filename_len);
+	entry->filename = filename_ptr;
 	entry->filename_len = (uint16_t)filename_len;
 
+	git_oid_cpy(&entry->oid, id);
+
 	return entry;
 }
 
@@ -231,7 +224,7 @@ int git_tree_entry_dup(git_tree_entry **dest, const git_tree_entry *source)
 
 	GIT_ASSERT_ARG(source);
 
-	cpy = alloc_entry(source->filename, source->filename_len, source->oid);
+	cpy = alloc_entry(source->filename, source->filename_len, &source->oid);
 	if (cpy == NULL)
 		return -1;
 
@@ -269,7 +262,7 @@ const char *git_tree_entry_name(const git_tree_entry *entry)
 const git_oid *git_tree_entry_id(const git_tree_entry *entry)
 {
 	GIT_ASSERT_ARG_WITH_RETVAL(entry, NULL);
-	return entry->oid;
+	return &entry->oid;
 }
 
 git_object_t git_tree_entry_type(const git_tree_entry *entry)
@@ -292,7 +285,7 @@ int git_tree_entry_to_object(
 	GIT_ASSERT_ARG(entry);
 	GIT_ASSERT_ARG(object_out);
 
-	return git_object_lookup(object_out, repo, entry->oid, GIT_OBJECT_ANY);
+	return git_object_lookup(object_out, repo, &entry->oid, GIT_OBJECT_ANY);
 }
 
 static const git_tree_entry *entry_fromname(
@@ -331,7 +324,7 @@ const git_tree_entry *git_tree_entry_byid(
 	GIT_ASSERT_ARG_WITH_RETVAL(tree, NULL);
 
 	git_array_foreach(tree->entries, i, e) {
-		if (memcmp(&e->oid->id, &id->id, sizeof(id->id)) == 0)
+		if (git_oid_equal(&e->oid, id))
 			return e;
 	}
 
@@ -432,7 +425,7 @@ int git_tree__parse_raw(void *_tree, const char *data, size_t size)
 			entry->attr = attr;
 			entry->filename_len = (uint16_t)filename_len;
 			entry->filename = buffer;
-			entry->oid = (git_oid *) ((char *) buffer + filename_len + 1);
+			git_oid_fromraw(&entry->oid, ((unsigned char *) buffer + filename_len + 1));
 		}
 
 		buffer += filename_len + 1;
@@ -536,7 +529,7 @@ static int git_treebuilder__write_with_buffer(
 
 		git_str_printf(buf, "%o ", entry->attr);
 		git_str_put(buf, entry->filename, entry->filename_len + 1);
-		git_str_put(buf, (char *)entry->oid->id, GIT_OID_RAWSZ);
+		git_str_put(buf, (char *)entry->oid.id, GIT_OID_RAWSZ);
 
 		if (git_str_oom(buf)) {
 			error = -1;
@@ -765,7 +758,7 @@ int git_treebuilder_new(
 		git_array_foreach(source->entries, i, entry_src) {
 			if (append_entry(
 				bld, entry_src->filename,
-				entry_src->oid,
+				&entry_src->oid,
 				entry_src->attr,
 				false) < 0)
 				goto on_error;
@@ -798,7 +791,7 @@ int git_treebuilder_insert(
 		return error;
 
 	if ((entry = git_strmap_get(bld->map, filename)) != NULL) {
-		git_oid_cpy((git_oid *) entry->oid, id);
+		git_oid_cpy(&entry->oid, id);
 	} else {
 		entry = alloc_entry(filename, strlen(filename), id);
 		GIT_ERROR_CHECK_ALLOC(entry);
@@ -954,7 +947,7 @@ int git_tree_entry_bypath(
 		return git_tree_entry_dup(entry_out, entry);
 	}
 
-	if (git_tree_lookup(&subtree, root->object.repo, entry->oid) < 0)
+	if (git_tree_lookup(&subtree, root->object.repo, &entry->oid) < 0)
 		return -1;
 
 	error = git_tree_entry_bypath(
@@ -995,7 +988,7 @@ static int tree_walk(
 			git_tree *subtree;
 			size_t path_len = git_str_len(path);
 
-			error = git_tree_lookup(&subtree, tree->object.repo, entry->oid);
+			error = git_tree_lookup(&subtree, tree->object.repo, &entry->oid);
 			if (error < 0)
 				break;
 
diff --git a/src/libgit2/tree.h b/src/libgit2/tree.h
index 6bd9ed6..0dd963f 100644
--- a/src/libgit2/tree.h
+++ b/src/libgit2/tree.h
@@ -19,7 +19,7 @@
 struct git_tree_entry {
 	uint16_t attr;
 	uint16_t filename_len;
-	const git_oid *oid;
+	git_oid oid;
 	const char *filename;
 };
 
diff --git a/tests/libgit2/iterator/tree.c b/tests/libgit2/iterator/tree.c
index 4145c8d..06a920a 100644
--- a/tests/libgit2/iterator/tree.c
+++ b/tests/libgit2/iterator/tree.c
@@ -268,7 +268,7 @@ static void check_tree_entry(
 
 	cl_git_pass(git_iterator_current_tree_entry(&te, i));
 	cl_assert(te);
-	cl_assert(git_oid_streq(te->oid, oid) == 0);
+	cl_assert(git_oid_streq(&te->oid, oid) == 0);
 
 	cl_git_pass(git_iterator_current(&ie, i));
 
diff --git a/tests/libgit2/object/tree/parse.c b/tests/libgit2/object/tree/parse.c
index 9d76a74..4e871dc 100644
--- a/tests/libgit2/object/tree/parse.c
+++ b/tests/libgit2/object/tree/parse.c
@@ -40,7 +40,7 @@ static void assert_tree_parses(const char *data, size_t datalen,
 		cl_assert(entry = git_tree_entry_byname(tree, expected->filename));
 		cl_assert_equal_s(expected->filename, entry->filename);
 		cl_assert_equal_i(expected->attr, entry->attr);
-		cl_assert_equal_oid(&oid, entry->oid);
+		cl_assert_equal_oid(&oid, &entry->oid);
 	}
 
 	git_object_free(&tree->object);