Subject: Re: [PATCH v2 14/34] sequencer (rebase -i): update refs after a successful rebase

From: Junio C Hamano <[email-masked]>
Date: Fri, 16 Dec 2016 11:19:04 -0800

Johannes Schindelin <[email-masked]> writes:

> An interactive rebase operates on a detached HEAD (to keep the reflog
> of the original branch relatively clean), and updates the branch only
> at the end.
>
> Now that the sequencer learns to perform interactive rebases, it also
> needs to learn the trick to update the branch before removing the
> directory containing the state of the interactive rebase.
>
> We introduce a new head_ref variable in a wider scope than necessary at
> the moment, to allow for a later patch that prints out "Successfully
> rebased and updated <ref>".
>
> Signed-off-by: Johannes Schindelin <[email-masked]>
> ---
>  sequencer.c | 32 +++++++++++++++++++++++++++++++-
>  1 file changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index a6625e765d..a4e9b326ba 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -100,6 +100,8 @@ static GIT_PATH_FUNC(rebase_path_stopped_sha, "rebase-merge/stopped-sha")
>  static GIT_PATH_FUNC(rebase_path_gpg_sign_opt, "rebase-merge/gpg_sign_opt")
>  static GIT_PATH_FUNC(rebase_path_orig_head, "rebase-merge/orig-head")
>  static GIT_PATH_FUNC(rebase_path_verbose, "rebase-merge/verbose")
> +static GIT_PATH_FUNC(rebase_path_head_name, "rebase-merge/head-name")
> +static GIT_PATH_FUNC(rebase_path_onto, "rebase-merge/onto")
>  
>  static inline int is_rebase_i(const struct replay_opts *opts)
>  {
> @@ -1793,12 +1795,39 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
>  	}
>  
>  	if (is_rebase_i(opts)) {
> -		struct strbuf buf = STRBUF_INIT;
> +		struct strbuf head_ref = STRBUF_INIT, buf = STRBUF_INIT;
>  
>  		/* Stopped in the middle, as planned? */
>  		if (todo_list->current < todo_list->nr)
>  			return 0;
>  
> +		if (read_oneliner(&head_ref, rebase_path_head_name(), 0) &&
> +				starts_with(head_ref.buf, "refs/")) {
> +			unsigned char head[20], orig[20];
> +
> +			if (get_sha1("HEAD", head))
> +				return error(_("cannot read HEAD"));
> +			if (!read_oneliner(&buf, rebase_path_orig_head(), 0) ||
> +					get_sha1_hex(buf.buf, orig))
> +				return error(_("could not read orig-head"));
> +			strbuf_addf(&buf, "rebase -i (finish): %s onto ",
> +				head_ref.buf);
> +			if (!read_oneliner(&buf, rebase_path_onto(), 0))
> +				return error(_("could not read 'onto'"));
> +			if (update_ref(buf.buf, head_ref.buf, head, orig,
> +					REF_NODEREF, UPDATE_REFS_MSG_ON_ERR))
> +				return error(_("could not update %s"),
> +					head_ref.buf);
> +			strbuf_reset(&buf);
> +			strbuf_addf(&buf,
> +				"rebase -i (finish): returning to %s",
> +				head_ref.buf);
> +			if (create_symref("HEAD", head_ref.buf, buf.buf))
> +				return error(_("could not update HEAD to %s"),
> +					head_ref.buf);

All of the above return error() calls leak head_ref.buf; in addition
some leak buf.buf, too.

> +			strbuf_reset(&buf);
> +		}
> +
>  		if (opts->verbose) {
>  			const char *argv[] = {
>  				"diff-tree", "--stat", NULL, NULL
> @@ -1813,6 +1842,7 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
>  			strbuf_reset(&buf);
>  		}
>  		strbuf_release(&buf);
> +		strbuf_release(&head_ref);
>  	}
>  
>  	/*