[v2,4/9] submodule: inline submodule_commits() into caller

Commit Message

Glen Choo Feb. 15, 2022, 5:23 p.m. UTC

When collecting the string_list of changed submodule names, the new
submodules commits are stored in the string_list_item.util as an
oid_array. A subsequent commit will replace the oid_array with a struct
that has more information.

Prepare for this change by inlining submodule_commits() (which inserts
into the string_list and initializes the string_list_item.util) into its
only caller so that the code is easier to refactor later.

Signed-off-by: Glen Choo <[e-mail]>
---
 submodule.c | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

Comments

Ævar Arnfjörð Bjarmason Feb. 15, 2022, 10:02 p.m. UTC | #1

On Wed, Feb 16 2022, Glen Choo wrote:

> -		commits = submodule_commits(changed, name);
> -		oid_array_append(commits, &p->two->oid);
> +		item = string_list_insert(changed, name);
> +		if (!item->util)
> +			/* NEEDSWORK: should we have oid_array_init()? */
> +			item->util = xcalloc(1, sizeof(struct oid_array));
> +		oid_array_append(item->util, &p->two->oid);
>  	}
>  }

Yes, just adding it while we're at it seems worthwhile, and if not
defining this in terms of the macro would be better, as the two are
guaranteed not to drift apart. I.e. the pattern seen in:

    git grep -W 'memcpy.*&blank'

Patch

diff --git a/submodule.c b/submodule.c
index 7032dcabb8..7fdf7793fb 100644
--- a/submodule.c
+++ b/submodule.c
@@ -782,19 +782,6 @@  const struct submodule *submodule_from_ce(const struct cache_entry *ce)
 	return submodule_from_path(the_repository, null_oid(), ce->name);
 }
 
-static struct oid_array *submodule_commits(struct string_list *submodules,
-					   const char *name)
-{
-	struct string_list_item *item;
-
-	item = string_list_insert(submodules, name);
-	if (item->util)
-		return (struct oid_array *) item->util;
-
-	/* NEEDSWORK: should we have oid_array_init()? */
-	item->util = xcalloc(1, sizeof(struct oid_array));
-	return (struct oid_array *) item->util;
-}
 
 struct collect_changed_submodules_cb_data {
 	struct repository *repo;
@@ -830,9 +817,9 @@  static void collect_changed_submodules_cb(struct diff_queue_struct *q,
 
 	for (i = 0; i < q->nr; i++) {
 		struct diff_filepair *p = q->queue[i];
-		struct oid_array *commits;
 		const struct submodule *submodule;
 		const char *name;
+		struct string_list_item *item;
 
 		if (!S_ISGITLINK(p->two->mode))
 			continue;
@@ -859,8 +846,11 @@  static void collect_changed_submodules_cb(struct diff_queue_struct *q,
 		if (!name)
 			continue;
 
-		commits = submodule_commits(changed, name);
-		oid_array_append(commits, &p->two->oid);
+		item = string_list_insert(changed, name);
+		if (!item->util)
+			/* NEEDSWORK: should we have oid_array_init()? */
+			item->util = xcalloc(1, sizeof(struct oid_array));
+		oid_array_append(item->util, &p->two->oid);
 	}
 }