"Jayati Shrivastava via GitGitGadget" <gitgitgadget@xxxxxxxxx>
writes:
> Subject: Re: [PATCH] Use reverse_commit_list helper function for in-place list reversal
Perhaps
Subject: [PATCH] sequencer: use reverse_commit_list_helper()
cf. Documentation/SubmittingPatches::[[summary-section]]
> From: JAYATI SHRIVASTAVA <gaurijove@xxxxxxxxx>
This name should match what is on Signed-off-by: line. Check your
user.name configuration, perhaps?
> Here, a reverse copy of a list is being created by iterating
> over the list after which the original list is discarded.
> Instead of creating a new allocation, we can reverse the
> original list in-place using the reverse_commit_list helper
> function.
Perhaps
Instead of creating a new allocation, reverse the list in-place
by calling the reverse_commit_list() helper.
cf. Documentation/SubmittingPatches::[[imperative-mood]]
>
> Signed-off-by: Jayati Shrivastava <gaurijove@xxxxxxxxx>
> ---
> @@ -3984,9 +3984,7 @@ static int do_merge(struct repository *r,
> git_path_merge_head(r), 0);
> write_message("no-ff", 5, git_path_merge_mode(r), 0);
>
> - for (j = bases; j; j = j->next)
> - commit_list_insert(j->item, &reversed);
> - free_commit_list(bases);
> + bases = reverse_commit_list(bases);
If the code in the original code that followed from here used both
"bases" and "reversed", this would not have worked, but because the
original gets rid of "bases", we can just reverse the list in-place
with reverse_commit_list() helper and reuse the same variable.
> repo_read_index(r);
> init_merge_options(&o, r);
> @@ -4002,10 +4000,10 @@ static int do_merge(struct repository *r,
> * update the index and working copy immediately.
> */
> ret = merge_ort_recursive(&o,
> - head_commit, merge_commit, reversed,
> + head_commit, merge_commit, bases,
> &i);
> } else {
> - ret = merge_recursive(&o, head_commit, merge_commit, reversed,
> + ret = merge_recursive(&o, head_commit, merge_commit, bases,
> &i);
> }
Looks good.
Thanks.
