Re: [PATCH v1 5/5] sequencer: directly call pick_commits() from complete_action()

["René Scharfe" <l.s.r@web.de>]

Am 25.09.19 um 22:13 schrieb Alban Gruin:
> Currently, complete_action() calls sequencer_continue() to do the
> rebase.  Even though the former already has the todo list, the latter
> loads it from the disk and parses it.  Calling directly pick_commits()
> from complete_action() avoids this unnecessary round trip.
>
> Signed-off-by: Alban Gruin <alban.gruin@gmail.com>
> ---
>  sequencer.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index ec7ea8d9e5..b395dd6e11 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -5140,15 +5140,17 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla
>  		return error_errno(_("could not write '%s'"), todo_file);
>  	}
>
> -	todo_list_release(&new_todo);
> -

Moving this call down leaks new_todo on error...

>  	if (checkout_onto(r, opts, onto_name, &oid, orig_head))
>  		return -1;

... here ...

>
>  	if (require_clean_work_tree(r, "rebase", "", 1, 1))
>  		return -1;
... and here.  Perhaps set res to -1 and jump to the end?

>
> -	return sequencer_continue(r, opts);
> +	todo_list_write_total_nr(&new_todo);
> +	res = pick_commits(r, &new_todo, opts);
> +	todo_list_release(&new_todo);
> +
> +	return res;
>  }
>
>  struct subject2item_entry {
>