Re: Segfault in git when using git logs

From: Jeff King <peff@peff.net>
Date: Nov 3, 2020, 18:57 UTC
Message-ID: <20201103185711.GA461461@coredump.intra.peff.net>
In-Reply-To: <xmqq361qs31a.fsf@gitster.c.googlers.com>

On Tue, Nov 03, 2020 at 10:34:09AM -0800, Junio C Hamano wrote:

Show 9 quoted lines

> > On our side, I don't think it would be _wrong_ to catch and disallow the
> > combination. But it may be nicer to them if we continue to quietly
> > ignore --follow and the pathspec in that case, for working with older
> > versions. (OTOH, if I understand correctly they're segfaulting every
> > time VS Code is used with v2.29 now, so they may have to accept it as an
> > urgent fix anyway).
> 
> So something like this won't harm VS Code more than we currently do,
> while telling users what is wrong with their command line?

Yeah, I was wondering if we'd want the patch you sent, or if we should turn those die() calls into warning() and disable the flags up front.

> We may still want the "silently disable follow" at low-level
> protection, but that does not give feedback why the end-user option
> is silently ignored, so...

I'd be just as happy to leave it out, if we think it isn't triggerable. The segfault would let people know we missed a spot. ;)

Show 11 quoted lines

> diff --git c/builtin/log.c w/builtin/log.c
> index 9f939e6cdf..8811084f02 100644
> --- c/builtin/log.c
> +++ w/builtin/log.c
> @@ -206,6 +206,13 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
>  	if (argc > 1)
>  		die(_("unrecognized argument: %s"), argv[1]);
>  
> +	if (rev->line_level_traverse) {
> +		if (rev->diffopt.filter)
> +			die(_("-L<range>:<file> cannot be used with pathspec"));

Should this be checking rev->diffopt.pathspec.nr?

I could well believe that --diff-filter=A does not work with "-L" either, but that is a separate story.

-Peff