Re: [PATCH v10 33/36] merge-recursive: fix was_tracked() to quit lying with some renamed paths
- From
Elijah Newren <newren@gmail.com>
- Date
- Apr 19, 2018, 20:54 UTC
- Message-ID
- <CABPp-BF9gjC-Jw=NZQygxwQ7d_dbZEUB2OoQ-ieSG6bNUWs5BA@mail.gmail.com>
- In-Reply-To
- <CAN0heSquJboMMgay+5XomqXCGoHtXxf1mJBmY_L7y+AA4eG0KA@mail.gmail.com>
On Thu, Apr 19, 2018 at 1:39 PM, Martin Ågren <martin.agren@gmail.com> wrote:
Show 12 quoted lines
> On 19 April 2018 at 19:58, Elijah Newren <newren@gmail.com> wrote: >> + /* Free the extra index left from git_merge_trees() */ >> + /* >> + * FIXME: Need to also data allocated by setup_unpack_trees_porcelain() >> + * tucked away in o->unpack_opts.msgs, but the problem is that only >> + * half of it refers to dynamically allocated data, while the other >> + * half points at static strings. >> + */ > > Timing. I've been preparing a patch that provides > `clear_unpack_trees_porcelain()` and fixes all such leaks. (About 10% of > all the leaks that are reported when I run the test-suite!) My patch
Nice!
Show 6 quoted lines
> conflicts with this series for obvious reasons. Figuring out the > conflict resolution might be non-trivial, and I suspect it would even be > an evil merge. I'll be holding off on that patch until this has landed. > > BTW: s/also data/also free data/. But since I'm promising to get rid of > this TODO quite soon after this is merged... ;-)
Oops, good catch. I can fix it up since I need to fix the issues SZEDER found, but yeah if you're just going to implement the fix and rip this comment out then it's not that critical.