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

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

[René Scharfe]

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(-)

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

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

[Junio C Hamano]

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

> 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.

> 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