Re: [PATCH 1/5] diff --color-moved-ws: fix double free crash

From: Phillip Wood <phillip.wood@talktalk.net>
Date: Oct 3, 2018, 09:38 UTC
Message-ID: <1c6b26a2-45ca-df76-8baa-61790cb963ac@talktalk.net>
In-Reply-To: <CAGZ79kYqACgoNShaPBQQSUN1B_dhmdxHxBV6YZoNAd39f6XMjw@mail.gmail.com>

Hi Stefan

Thanks for looking at these patches

On 02/10/2018 19:49, Stefan Beller wrote:

Show 28 quoted lines

> On Tue, Oct 2, 2018 at 10:55 AM Phillip Wood <phillip.wood@talktalk.net> wrote:
> 
>> The solution is to store the ws_delta in the array of potential moved
>> blocks rather than with the lines. This means that it no longer needs
>> to be copied around and one block cannot overwrite the ws_delta of
>> another. Additionally it saves some malloc/free calls as we don't keep
>> allocating and freeing ws_deltas.
> 
> Another solution would be to duplicate the copy-arounds, that it only
> fixes the double free, but having another layer of abstraction
> (moved block vs line) makes sense as then we don't need to copy
> it forward.
> 
> With this patch applied the diff as mentioned works and having the
> ws deltas with the blocks instead of the
> 
>>
>> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> 
> 
> 
>>  static void pmb_advance_or_null_multi_match(struct diff_options *o,
> [...]
>>         for (i = 0; i < pmb_nr; i++) {
>>                 if (got_match[i]) {
>>                         /* Carry the white space delta forward */
> 
> I would think this comment is obsolete as well with this patch?

Yes you're right I should have removed that. As there are some changes needed to some other comments I'll re-roll

Best Wishes

Phillip

> 
> With or without that nit addressed, this patch is
> Reviewed-by: Stefan Beller <sbeller@google.com>
>