Re: [PATCH v2 2/3] config.c: don't leak memory in handle_path_include()

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Fri, Oct 22 2021, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>>> Not a problem introduced by this function, but if you look at this
>>> change with "git show -W", we'd notice that the function name on the
>>> hunk header looks strange.  I think we should add a blank line
>>> before the beginning of the function.
>>
>> I think this is a bug in -W, after all if without it we we show the
>> function context line, but with it we advance further, then that means
>> that -W didn't find the correct function boundary.
>
> That's a chicken-and-egg argument, and I do not think it is a bug in
> "-W" nor the funcname regular expression pattern we use.  We expect
> a blank line there and the pattern reflects that expectation, so not
> having an expected blank line is what causes this problem.
>
> In any case, we should add a blank linke before the beginning of the
> function, and of course that is obviously outside the scope of these
> patches.

Sort of, if you were running with the patch I posted at [1] you wouldn't
see the bad value at @@, but we still extend upwards with -W, which I
consider a bug.

I.e. both the current context we display and the over-extension there is
ultimately a symptom of the same issue, which is that what we're doing
with -W gets conflated with behavior that makes sense without -W, notice
how if you do "git log -W" on anything that the @@ context we display is
the prototype of the function /above/ the one you're likely looking at
the code change in.

So the blank line is the cause of the over-extension, but we'd still
show the (IMO) incorrect context in either case.

Anyway, as you say a discussion for some other thread. I've been meaning
to get back to those patches at some point, the first problem is that
our test coverage for what function context we should find when is
really lacking, so any changes in that part of the xdiff code are likely
to break things. I had those tests, but they got lost in some
bikeshedding...

1. https://lore.kernel.org/git/20210215155020.2804-2-avarab@gmail.com/