[PATCH v3] sequencer.c: use reverse_commit_list() helper

From: Jayati Shrivastava via GitGitGadget <gitgitgadget@gmail.com>
Date: Mar 16, 2022, 11:20 UTC
Message-ID: <pull.1177.v3.git.1647429623830.gitgitgadget@gmail.com>
In-Reply-To: <pull.1177.v2.git.1647427356490.gitgitgadget@gmail.com>

From: Jayati Shrivastava <gaurijove@gmail.com>

Instead of creating a new allocation, reverse the original list in-place by calling the reverse_commit_list() helper. The original code discards the list "bases" after storing its reverse copy in a newly created list "reversed". If the code that followed from here used both "bases" and "reversed", the modification would not have worked, but since the original list "bases" gets discarded, we can simply reverse "bases" in-place with the reverse_commit_list() helper and reuse the same variable in the code that follows.

builtin/merge.c has been left unmodified, since in its case, the original list is needed separately from its reverse copy by the code.

Signed-off-by: Jayati Shrivastava <gaurijove@gmail.com>
---
    Use reverse_commit_list helper function for in-place list reversal
    
    This patch address https://github.com/gitgitgadget/git/issues/1156
    Changes since v2:
    
     * updated the commit message to explain why builtin/merge.c has not
       been modified

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1177%2Fvictorphoenix3%2Freverse-list-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1177/victorphoenix3/reverse-list-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1177

Range-diff vs v2:

 1:  f794bcd8bda ! 1:  6dd0d92096b sequencer.c: use reverse_commit_list() helper
     @@ Metadata
       ## Commit message ##
          sequencer.c: use reverse_commit_list() helper
      
     -    Instead of creating a new allocation, reverse the original
     -    list in-place by calling the reverse_commit_list() helper.
     -    The original code discards the list "bases" after storing
     -    its reverse copy in a newly created list "reversed". If the
     -    code that followed from here used both "bases" and "reversed",
     -    the modification would not have worked, but since the original
     -    list "bases" gets discarded, we can simply reverse "bases"
     -    in-place with the reverse_commit_list() helper and reuse the
     -    same variable in the code that follows.
     +    Instead of creating a new allocation, reverse the
     +    original list in-place by calling the
     +    reverse_commit_list() helper. The original code
     +    discards the list "bases" after storing its
     +    reverse copy in a newly created list "reversed".
     +    If the code that followed from here used both
     +    "bases" and "reversed", the modification would not
     +    have worked, but since the original list "bases"
     +    gets discarded, we can simply reverse "bases"
     +    in-place with the reverse_commit_list() helper and
     +    reuse the same variable in the code that follows.
     +
     +    builtin/merge.c has been left unmodified, since in
     +    its case, the original list is needed separately
     +    from its reverse copy by the code.
      
          Signed-off-by: Jayati Shrivastava <gaurijove@gmail.com>

 sequencer.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 35006c0cea6..bccbb9e3522 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3749,7 +3749,7 @@ static int do_merge(struct repository *r,
 	int run_commit_flags = 0;
 	struct strbuf ref_name = STRBUF_INIT;
 	struct commit *head_commit, *merge_commit, *i;
-	struct commit_list *bases, *j, *reversed = NULL;
+	struct commit_list *bases, *j;
 	struct commit_list *to_merge = NULL, **tail = &to_merge;
 	const char *strategy = !opts->xopts_nr &&
 		(!opts->strategy ||
@@ -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);
 
 	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);
 	}
 	if (ret <= 0)

base-commit: b896f729e240d250cf56899e6a0073f6aa469f5d

-- 
gitgitgadget