[PATCH 0/2] diffcore-rename with duplicate tree entries can segfault

From: Jeff King <peff@peff.net>
Date: Feb 27, 2015, 01:38 UTC
Message-ID: <20150227013847.GA2983@peff.net>
In-Reply-To: <xmqq1tldoe35.fsf@gitster.dls.corp.google.com>

On Wed, Feb 25, 2015 at 01:50:38PM -0800, Junio C Hamano wrote:

Show 14 quoted lines

> > So to go forward, I'm happy to prepare a patch, but I'd like to know:
> >
> >   1. Does something like the above look reasonable to you (I'd probably
> >      refactor it to avoid the bizarre return value semantics from
> >      locate_rename_dst, though)?
> >
> >   2. If so, do you want something minimal like what's above, or do you
> >      mind if I build it on top of a hashmap conversion? I suspect the
> >      logic may also end up more clear with the hashmap (since inserting
> >      versus lookup will be more distinct in the callers).
> 
> No, I don't mind.  The diff-b-m topic seems to need a lot deeper
> rethink than I originally anticipated anyway, and it can wait for a
> clean-up to use hashmap to stabilize.

I tried switching to a hashmap, but the diff_score code actually wants to index into the array using an int. In a hashmap, we'd use a pointer instead, but that increases the size of "struct diff_score", which is something that we have to allocate a lot of (src * dst, I think).

So I punted on that and just cleaned up the locate_rename_dst interface a bit. Here's the result.

  [1/2]: diffcore-rename: split locate_rename_dst into two functions
  [2/2]: diffcore-rename: avoid processing duplicate destinations

-Peff