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ń
