From: Rohit Ashiwal <rohit.ashiwal265@gmail.com>
Date: Sun, 06 Oct 2019 17:57:29 GMT
Subject: Re: [PATCH v4 3/6] rebase -i: support --committer-date-is-author-date
Message-ID: <20191006175729.17257-1-rohit.ashiwal265@gmail.com>
In-Reply-To: <3f29cb5d-dbc2-ae00-651b-22e9f437372a@gmail.com>

On Fri, 4 Oct 2019 10:37:43 +0100 Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> > +	if (opts->committer_date_is_author_date) {
> > +		int len = strlen(author);
> > +		struct ident_split ident;
> > +		struct strbuf date = STRBUF_INIT;
> > +
> > +		if (split_ident_line(&ident, author, len) < 0)
> > +			return error(_("malformed ident line"));
>
> Here (and just below) we return if there is an error but later on we 
> `goto out` should we be doing that here to clean something up?

Yes, a mistake indeed. This should be replace with an error and a
goto statement.

> Best Wishes
>
> Phillip
>
> > +		if (!ident.date_begin)
> > +			return error(_("corrupted author without date information"));
> > +
> > +		strbuf_addf(&date, "@%.*s %.*s",
> > +			    (int)(ident.date_end - ident.date_begin), ident.date_begin,
> > +			    (int)(ident.tz_end - ident.tz_begin), ident.tz_begin);
> > +		res = setenv("GIT_COMMITTER_DATE", date.buf, 1);
> > +		strbuf_release(&date);
> > +
> > +		if (res)
> > +			goto out;
> > +	}
> > +

Thanks
Rohit