[v2,13/17] mktree: use iterator struct to add tree entries to index

Commit Message

Victoria Dye June 19, 2024, 9:58 p.m. UTC

From: Victoria Dye <[e-mail]>

Create 'struct tree_entry_iterator' to manage iteration through a 'struct
tree_entry_array'. Using an iterator allows for conditional iteration; this
functionality will be necessary in later commits when performing parallel
iteration through multiple sets of tree entries.

Signed-off-by: Victoria Dye <[e-mail]>
---
 builtin/mktree.c | 39 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 36 insertions(+), 3 deletions(-)

Comments

Junio C Hamano June 26, 2024, 9:10 p.m. UTC | #1

"Victoria Dye via GitGitGadget" <[e-mail]> writes:

> @@ -157,14 +186,18 @@ static int add_tree_entry_to_index(struct index_state *istate,
>  
>  static void write_tree(struct tree_entry_array *arr, struct object_id *oid)
>  {
> +	struct tree_entry_iterator iter = { NULL };
>  	struct index_state istate = INDEX_STATE_INIT(the_repository);
>  	istate.sparse_index = 1;
>  
>  	sort_and_dedup_tree_entry_array(arr);
>  
> -	/* Construct an in-memory index from the provided entries */
> -	for (size_t i = 0; i < arr->nr; i++) {
> -		struct tree_entry *ent = arr->entries[i];
> +	tree_entry_iterator_init(&iter, arr);
> +
> +	/* Construct an in-memory index from the provided entries & base tree */
> +	while (iter.current) {
> +		struct tree_entry *ent = iter.current;
> +		tree_entry_iterator_advance(&iter);
>  
>  		if (add_tree_entry_to_index(&istate, ent))
>  			die(_("failed to add tree entry '%s'"), ent->name);

OK, looking good.

If we make _iterator_init() and _iterator_advance to both return the
current, then the loop can still be like so:

	for (ent = tree_entry_iterator_init(&iter, arr);
             ent;
	     ent = tree_entry_iterator_advance(&iter)) {
		... use ent ...
	}

and .current does not need to be a non-private member, if we wanted
to (I am not convinced it is necessarily a better interface to make
.current as private---especially if we end up needing _peek() method
to learn its value, i.e. the value the most recent call to _init()
or _advance() returned.  If we need a write access to .current from
outside the interator interface, then what I outlined above would
not be a good match).


Thanks.

Patch

diff --git a/builtin/mktree.c b/builtin/mktree.c
index 3ce8d3dc524..344c9b9b6fe 100644
--- a/builtin/mktree.c
+++ b/builtin/mktree.c
@@ -139,6 +139,35 @@  static void sort_and_dedup_tree_entry_array(struct tree_entry_array *arr)
 	QSORT_S(arr->entries, arr->nr, ent_compare, &ignore_mode);
 }
 
+struct tree_entry_iterator {
+	struct tree_entry *current;
+
+	/* private */
+	struct {
+		struct tree_entry_array *arr;
+		size_t idx;
+	} priv;
+};
+
+static void tree_entry_iterator_init(struct tree_entry_iterator *iter,
+				     struct tree_entry_array *arr)
+{
+	iter->priv.arr = arr;
+	iter->priv.idx = 0;
+	iter->current = 0 < arr->nr ? arr->entries[0] : NULL;
+}
+
+/*
+ * Advance the tree entry iterator to the next entry in the array. If no
+ * entries remain, 'current' is set to NULL.
+ */
+static void tree_entry_iterator_advance(struct tree_entry_iterator *iter)
+{
+	iter->current = (iter->priv.idx + 1) < iter->priv.arr->nr
+			? iter->priv.arr->entries[++iter->priv.idx]
+			: NULL;
+}
+
 static int add_tree_entry_to_index(struct index_state *istate,
 				   struct tree_entry *ent)
 {
@@ -157,14 +186,18 @@  static int add_tree_entry_to_index(struct index_state *istate,
 
 static void write_tree(struct tree_entry_array *arr, struct object_id *oid)
 {
+	struct tree_entry_iterator iter = { NULL };
 	struct index_state istate = INDEX_STATE_INIT(the_repository);
 	istate.sparse_index = 1;
 
 	sort_and_dedup_tree_entry_array(arr);
 
-	/* Construct an in-memory index from the provided entries */
-	for (size_t i = 0; i < arr->nr; i++) {
-		struct tree_entry *ent = arr->entries[i];
+	tree_entry_iterator_init(&iter, arr);
+
+	/* Construct an in-memory index from the provided entries & base tree */
+	while (iter.current) {
+		struct tree_entry *ent = iter.current;
+		tree_entry_iterator_advance(&iter);
 
 		if (add_tree_entry_to_index(&istate, ent))
 			die(_("failed to add tree entry '%s'"), ent->name);