Re: [PATCH 0/2] fast-import input string handling bugs

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 26, 2019, 15:36 UTC
Message-ID: <xmqqo90cj5w2.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20190825080640.GA31453@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 14 quoted lines

> On Sun, Aug 25, 2019 at 02:57:48AM -0400, Jeff King wrote:
>
>> And I think this is actually a real bug in the current code! We keep a
>> pointer to the encoding string, which survives because of the history.
>> But that history is bounded, and we could have an indefinite number of
>> changed files in the middle. If I modify t9300 like this:
>
> Here are two patches. The first fixes the existing bug with "encoding",
> and the second uses the approach I suggested to fix the leak you
> noticed.
>
> The second one does carry a greater risk of regression than your patch,
> but I think it's worth it for the fact that it makes any other bugs
> (like the "encoding" one) more obvious.

Yeah, it may be worth the risk, given that this is quite early in the cycle, so we have enough time to cook it in 'next' to see if somebody screams ;-)

Show 8 quoted lines

>
>   [1/2]: fast-import: duplicate parsed encoding string
>   [2/2]: fast-import: duplicate into history rather than passing ownership
>
>  fast-import.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> -Peff