Re: [PATCH v3] diff: support anchoring line(s)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux