Re: [PATCH 1/1] Use reverse_commit_list helper function for in-place list reversal

Jayati Shrivastava· Mar 14, 2022, 12:03 UTC · lore

This patch addresses https://github.com/gitgitgadget/git/issues/1156 . I have left builtin/merge.c unmodified since in its case, the original list is needed separately from its reverse copy.

On Mon, Mar 14, 2022 at 1:36 PM JAYATI SHRIVASTAVA <gaurijove@gmail.com> wrote:

Show 53 quoted lines

>
> From: JAYATI SHRIVASTAVA <gaurijove@gmail.com>
>
> Here, a reverse copy of the list is being created by looping and the
> original list is no longer used by the code. Therefore, instead of creating a copy,
> we can reverse the original list in-place using the reverse_commit_list helper
> function a copy.
>
> 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 35006c0cea..bccbb9e352 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)
> --
> 2.35.1.455.g1a4874565f
>

Johannes Schindelin· Mar 16, 2022, 09:29 UTC · re: Jayati Shrivastava · lore

Hi Jayati,

On Mon, 14 Mar 2022, Jayati Shrivastava wrote:

> This patch addresses https://github.com/gitgitgadget/git/issues/1156 . I have
> left builtin/merge.c unmodified since in its case, the original list
> is needed separately
> from its reverse copy.

This information is quite useful. Can you please integrate it into the commit message before sending the next iteration?

Thank you, Dscho

Jayati Shrivastava· Mar 16, 2022, 11:32 UTC · re: Johannes Schindelin · lore

Hi Dsho,

Thanks for reviewing.

On Wed, Mar 16, 2022 at 2:59 PM Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

> This information is quite useful. Can you please integrate it into the
> commit message before sending the next iteration?

I have incorporated the changes here : https://lore.kernel.org/git/pull.1177.v3.git.1647429623830.gitgitgadget@gmail.com/ . I had to shift to another thread using Gitgadget since the patches that I am mailing using git send-email were not getting delivered to the public mailing list

patchRe: [PATCH 1/1] Use reverse_commit_list helper function for in-place list reversal