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

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Junio,

On Fri, 16 Dec 2016, Junio C Hamano wrote:

> Johannes Schindelin <johannes.schindelin@gmx.de> 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 <johannes.schindelin@gmx.de>
> > ---
> >  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.

Thanks, fixed.

Ciao,
Dscho