[PATCH] commit: move clear_commit_marks_many() loop body to clear_commit_marks()

René Scharfe· Mar 23, 2025, 09:53 UTC · lore

clear_commit_marks_many() clears multiple commits one by one. Move the code for handling a single commit to clear_commit_marks() and call it instead of the other way around, to simplify the code.

Signed-off-by: René Scharfe <l.s.r@web.de>
---
 commit.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

Show changes to commit.c +7 −10

diff --git a/commit.c b/commit.c
index 6efdb03997..425503bb9f 100644
--- a/commit.c
+++ b/commit.c
@@ -780,19 +780,17 @@ static void clear_commit_marks_1(struct commit_list **plist,

 void clear_commit_marks_many(size_t nr, struct commit **commit, unsigned int mark)
 {
-	for (size_t i = 0; i < nr; i++) {
-		struct commit_list *list = NULL;
-
-		clear_commit_marks_1(&list, *commit, mark);
-		while (list)
-			clear_commit_marks_1(&list, pop_commit(&list), mark);
-		commit++;
-	}
+	for (size_t i = 0; i < nr; i++)
+		clear_commit_marks(commit[i], mark);
 }

 void clear_commit_marks(struct commit *commit, unsigned int mark)
 {
-	clear_commit_marks_many(1, &commit, mark);
+	struct commit_list *list = NULL;
+
+	clear_commit_marks_1(&list, commit, mark);
+	while (list)
+		clear_commit_marks_1(&list, pop_commit(&list), mark);
 }

 struct commit *pop_commit(struct commit_list **stack)
--
2.49.0

Junio C Hamano· Mar 24, 2025, 05:50 UTC · re: René Scharfe · lore

Re: [PATCH] commit: move clear_commit_marks_many() loop body to clear_commit_marks()

René Scharfe <l.s.r@web.de> writes:

Show 8 quoted lines

> clear_commit_marks_many() clears multiple commits one by one.  Move the
> code for handling a single commit to clear_commit_marks() and call it
> instead of the other way around, to simplify the code.
>
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
>  commit.c | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)

I _suspect_ that the original motivation of the current code layout was hoping that with more information at hand, the *_many() variant could be optimized to work on multiple things at once, which may be more efficient than doing things one-by-one, but such an optimization never materialized.

And it is much simpler and easier to read what goes on in which function if these two functions are rewritten the way you just did. It is crystal clear that there is nothing clever in the *_many() variant.

Thanks.

Show 33 quoted lines

> diff --git a/commit.c b/commit.c
> index 6efdb03997..425503bb9f 100644
> --- a/commit.c
> +++ b/commit.c
> @@ -780,19 +780,17 @@ static void clear_commit_marks_1(struct commit_list **plist,
>
>  void clear_commit_marks_many(size_t nr, struct commit **commit, unsigned int mark)
>  {
> -	for (size_t i = 0; i < nr; i++) {
> -		struct commit_list *list = NULL;
> -
> -		clear_commit_marks_1(&list, *commit, mark);
> -		while (list)
> -			clear_commit_marks_1(&list, pop_commit(&list), mark);
> -		commit++;
> -	}
> +	for (size_t i = 0; i < nr; i++)
> +		clear_commit_marks(commit[i], mark);
>  }
>
>  void clear_commit_marks(struct commit *commit, unsigned int mark)
>  {
> -	clear_commit_marks_many(1, &commit, mark);
> +	struct commit_list *list = NULL;
> +
> +	clear_commit_marks_1(&list, commit, mark);
> +	while (list)
> +		clear_commit_marks_1(&list, pop_commit(&list), mark);
>  }
>
>  struct commit *pop_commit(struct commit_list **stack)
> --
> 2.49.0

patchcommit: move clear_commit_marks_many() loop body to clear_commit_marks()