Re: [PATCH v3 2/2] diff: add -I<regex> that ignores matching changes

["Michał Kępień" <michal@isc.org>]

First, thanks to Phillip for testing this.

> Yes, this is a good location to clear the diff_queue, which is
> per-invocation.  But diff_options->ignore_regex[] can and should be
> shared across multiple invocations of the diff machinery, as we are
> parsing upfront in the command line option parser just once to be
> used throughout the life of the program.  It is wrong to free them
> early like this.

Ugh, sorry, I am only now starting to see the big picture.

> I really hate to suggest this, one common API call made into the
> diff machinery from various consumers, when they are absolutely done
> with diff_options, is probably diff_result_code().  Its purpose is
> to collect bits computed to participate in the overall result into
> the final result code and anybody who ran one or more diff and wants
> to learn the outcome must call it, so it is unlikely that callers
> that matter are missing a call to finalize their uses of the diff
> machinery.  So if freeing .ignore_regex[] is really necessary, it
> could be used to tell us where to do so [*1*].
> 
> Unlike per-invocation resources that MUST be freed for repeated
> invocations of the diff machinery made in "git log" family,
> resources held for and repeatedly used during the entire session
> like this may not have to be freed, to be honest, though.

After reading your message, taking a closer look, and doing a few
Valgrind runs, I tend to agree that perhaps there is no need to free the
'ignore_regex' array after all - it does *not* get allocated repeatedly
during each invocation of the diff machinery and when Git exits, the OS
will reclaim any memory allocated by the process anyway.  On my laptop,
a Valgrind run done for a -DSUPPRESS_ANNOTATED_LEAKS build claims that
'ignore_regex' is one of 1,300+ memory blocks still reachable at exit
time, which amount to about 2.5 MB of memory in total.  Given this, it
feels slightly weird to single 'ignore_regex' out.

I will revert the diff_flush() changes in v4 unless somebody disagrees
with the above reasoning.

-- 
Best regards,
Michał Kępień