{"thread":{"id":"63179","subject":"[PATCH] commit: move clear_commit_marks_many() loop body to clear_commit_marks()","startedAt":"2025-03-23T09:53:23Z","lastAt":"2025-03-24T05:50:57Z","messageCount":2,"participants":["René Scharfe","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"514873","messageId":"80bfd7a9-904c-49d8-a367-ca268c096a9f@web.de","threadId":"63179","inReplyTo":null,"subject":"[PATCH] commit: move clear_commit_marks_many() loop body to clear_commit_marks()","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2025-03-23T09:53:21Z","receivedAt":"2025-03-23T09:53:23Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"clear_commit_marks_many() clears multiple commits one by one.  Move the\ncode for handling a single commit to clear_commit_marks() and call it\ninstead of the other way around, to simplify the code.\n\nSigned-off-by: René Scharfe <l.s.r@web.de>\n---\n commit.c | 16 +++++++---------\n 1 file changed, 7 insertions(+), 9 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex 6efdb03997..425503bb9f 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -780,19 +780,17 @@ static void clear_commit_marks_1(struct commit_list **plist,\n\n void clear_commit_marks_many(size_t nr, struct commit **commit, unsigned int mark)\n {\n-\tfor (size_t i = 0; i < nr; i++) {\n-\t\tstruct commit_list *list = NULL;\n-\n-\t\tclear_commit_marks_1(&list, *commit, mark);\n-\t\twhile (list)\n-\t\t\tclear_commit_marks_1(&list, pop_commit(&list), mark);\n-\t\tcommit++;\n-\t}\n+\tfor (size_t i = 0; i < nr; i++)\n+\t\tclear_commit_marks(commit[i], mark);\n }\n\n void clear_commit_marks(struct commit *commit, unsigned int mark)\n {\n-\tclear_commit_marks_many(1, &commit, mark);\n+\tstruct commit_list *list = NULL;\n+\n+\tclear_commit_marks_1(&list, commit, mark);\n+\twhile (list)\n+\t\tclear_commit_marks_1(&list, pop_commit(&list), mark);\n }\n\n struct commit *pop_commit(struct commit_list **stack)\n--\n2.49.0\n"},{"id":"514901","messageId":"xmqqo6xrou2p.fsf@gitster.g","threadId":"63179","inReplyTo":"80bfd7a9-904c-49d8-a367-ca268c096a9f@web.de","subject":"Re: [PATCH] commit: move clear_commit_marks_many() loop body to clear_commit_marks()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-24T05:50:54Z","receivedAt":"2025-03-24T05:50:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <l.s.r@web.de> writes:\n\n> clear_commit_marks_many() clears multiple commits one by one.  Move the\n> code for handling a single commit to clear_commit_marks() and call it\n> instead of the other way around, to simplify the code.\n>\n> Signed-off-by: René Scharfe <l.s.r@web.de>\n> ---\n>  commit.c | 16 +++++++---------\n>  1 file changed, 7 insertions(+), 9 deletions(-)\n\nI _suspect_ that the original motivation of the current code layout\nwas hoping that with more information at hand, the *_many() variant\ncould be optimized to work on multiple things at once, which may be\nmore efficient than doing things one-by-one, but such an optimization\nnever materialized.\n\nAnd it is much simpler and easier to read what goes on in which\nfunction if these two functions are rewritten the way you just did.\nIt is crystal clear that there is nothing clever in the *_many()\nvariant.\n\nThanks.\n\n> diff --git a/commit.c b/commit.c\n> index 6efdb03997..425503bb9f 100644\n> --- a/commit.c\n> +++ b/commit.c\n> @@ -780,19 +780,17 @@ static void clear_commit_marks_1(struct commit_list **plist,\n>\n>  void clear_commit_marks_many(size_t nr, struct commit **commit, unsigned int mark)\n>  {\n> -\tfor (size_t i = 0; i < nr; i++) {\n> -\t\tstruct commit_list *list = NULL;\n> -\n> -\t\tclear_commit_marks_1(&list, *commit, mark);\n> -\t\twhile (list)\n> -\t\t\tclear_commit_marks_1(&list, pop_commit(&list), mark);\n> -\t\tcommit++;\n> -\t}\n> +\tfor (size_t i = 0; i < nr; i++)\n> +\t\tclear_commit_marks(commit[i], mark);\n>  }\n>\n>  void clear_commit_marks(struct commit *commit, unsigned int mark)\n>  {\n> -\tclear_commit_marks_many(1, &commit, mark);\n> +\tstruct commit_list *list = NULL;\n> +\n> +\tclear_commit_marks_1(&list, commit, mark);\n> +\twhile (list)\n> +\t\tclear_commit_marks_1(&list, pop_commit(&list), mark);\n>  }\n>\n>  struct commit *pop_commit(struct commit_list **stack)\n> --\n> 2.49.0\n"}]}