Re: [GSoC][PATCH 1/1] rebase -i: add --ignore-whitespace flag

From: Rohit Ashiwal <rohit.ashiwal265@gmail.com>
Date: Jul 15, 2019, 22:00 UTC
Message-ID: <20190715220010.8146-1-rohit.ashiwal265@gmail.com>
In-Reply-To: <xmqqlfwz41vl.fsf@gitster-ct.c.googlers.com>

Hi Junio

On Mon, 15 Jul 2019 10:57:18 -0700 Junio C Hamano <gitster@pobox.com> wrote:

Show 13 quoted lines

> 
> Rohit Ashiwal <rohit.ashiwal265@gmail.com> writes:
> 
> > +	if (opts->ignore_whitespace) {
> > +		struct strbuf buf = STRBUF_INIT;
> > +
> > +		if (opts->strategy_opts)
> > +			strbuf_addstr(&buf, opts->strategy_opts);
> > +
> > +		strbuf_addstr(&buf, " --ignore-space-change");
> > +		free(opts->strategy_opts);
> 
> Is this call to free() safe?

Yes, as far as I can tell. Since up till now, `strategy_opts` is either "NULL" or xstrdup()'d string. There is no double free involved.

Show 12 quoted lines

> > +		opts->strategy_opts = strbuf_detach(&buf, NULL);
> > +	}
> > +
> > @@ -1821,6 +1836,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
> >  	}
> >
> >  	if (options.rebase_merges) {
> > +		if (options.ignore_whitespace)
> > +			die(_("cannot combine '--rebase-merges' with "
> > +			      "'--ignore-whitespace'"));
> 
> Hmph, this is unfortunate.  The patch is not making things worse, though.

Actually, there is no need for --rebase-merges and --ignore-whitespace to be "incompatible". My mentors also agree on this one. Making them work in harmony will be handled in another patch series.

Best Rohit