On Thu, Nov 30, 2017 at 3:26 PM, Jonathan Tan <jonathantanmy@xxxxxxxxxx> wrote:
> On Thu, 30 Nov 2017 01:36:37 +0100 (CET)
> Johannes Schindelin <Johannes.Schindelin@xxxxxx> wrote:
>
>> Hi Jonathan,
>>
>> On Tue, 28 Nov 2017, Jonathan Tan wrote:
>>
>> > @@ -4607,7 +4627,14 @@ int diff_opt_parse(struct diff_options *options,
>> > DIFF_XDL_CLR(options, NEED_MINIMAL);
>> > options->xdl_opts &= ~XDF_DIFF_ALGORITHM_MASK;
>> > options->xdl_opts |= value;
>> > + if (value == XDF_PATIENCE_DIFF)
>> > + clear_patience_anchors(options);
>> > return argcount;
>> > + } else if (skip_prefix(arg, "--anchored=", &arg)) {
>> > + options->xdl_opts = DIFF_WITH_ALG(options, PATIENCE_DIFF);
>> > + ALLOC_GROW(options->anchors, options->anchors_nr + 1,
>> > + options->anchors_alloc);
>> > + options->anchors[options->anchors_nr++] = xstrdup(arg);
>>
>> I looked and failed to find the code that releases this array after the
>> diff is done... did I miss anything?
>
> You didn't miss anything. As far as I can tell, occurrences of struct
> diff_options live throughout the lifetime of an invocation of Git and
> are not freed. (Even if the struct itself is allocated on the stack, its
> pathspec has some elements allocated on the heap.)
I thought at least for the intra-line word diff, which allocates its own
diff options struct, we'd clear them, but looking around we seem to leak
the diff options consistently. (builtin/blame.c is nice enough to
`clear_pathspec(&diff_opts.pathspec)`, but that's about it, no other
command takes care of the cruft.
I wonder if diff_flush might be a good place to clean up the diff options
and after the flush the diff options are declared invalid?
> test_expect_success '--anchored with non-unique line has no effect'
okay, if it turns out we need this case in the future we can still come up
with ideas.
Thanks,
Stefan
