Merge branch 'ab/diff-deferred-free'

A small memleak in "diff -I<regexp>" has been corrected.

* ab/diff-deferred-free:
  diff: plug memory leak from regcomp() on {log,diff} -I
  diff: add an API for deferred freeing
This commit is contained in:
Junio C Hamano 2021-02-22 16:12:43 -08:00
commit 45df6c4d75
4 changed files with 60 additions and 20 deletions

View File

@ -307,10 +307,11 @@ static struct itimerval early_output_timer;
static void log_show_early(struct rev_info *revs, struct commit_list *list)
{
int i = revs->early_output, close_file = revs->diffopt.close_file;
int i = revs->early_output;
int show_header = 1;
int no_free = revs->diffopt.no_free;
revs->diffopt.close_file = 0;
revs->diffopt.no_free = 0;
sort_in_topological_order(&list, revs->sort_order);
while (list && i) {
struct commit *commit = list->item;
@ -327,8 +328,8 @@ static void log_show_early(struct rev_info *revs, struct commit_list *list)
case commit_ignore:
break;
case commit_error:
if (close_file)
fclose(revs->diffopt.file);
revs->diffopt.no_free = no_free;
diff_free(&revs->diffopt);
return;
}
list = list->next;
@ -336,8 +337,8 @@ static void log_show_early(struct rev_info *revs, struct commit_list *list)
/* Did we already get enough commits for the early output? */
if (!i) {
if (close_file)
fclose(revs->diffopt.file);
revs->diffopt.no_free = 0;
diff_free(&revs->diffopt);
return;
}
@ -401,7 +402,7 @@ static int cmd_log_walk(struct rev_info *rev)
{
struct commit *commit;
int saved_nrl = 0;
int saved_dcctc = 0, close_file = rev->diffopt.close_file;
int saved_dcctc = 0;
if (rev->early_output)
setup_early_output();
@ -417,7 +418,7 @@ static int cmd_log_walk(struct rev_info *rev)
* and HAS_CHANGES being accumulated in rev->diffopt, so be careful to
* retain that state information if replacing rev->diffopt in this loop
*/
rev->diffopt.close_file = 0;
rev->diffopt.no_free = 1;
while ((commit = get_revision(rev)) != NULL) {
if (!log_tree_commit(rev, commit) && rev->max_count >= 0)
/*
@ -442,8 +443,8 @@ static int cmd_log_walk(struct rev_info *rev)
}
rev->diffopt.degraded_cc_to_c = saved_dcctc;
rev->diffopt.needed_rename_limit = saved_nrl;
if (close_file)
fclose(rev->diffopt.file);
rev->diffopt.no_free = 0;
diff_free(&rev->diffopt);
if (rev->diffopt.output_format & DIFF_FORMAT_CHECKDIFF &&
rev->diffopt.flags.check_failed) {
@ -1961,7 +1962,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
* file, but but we must instruct it not to close after each
* diff.
*/
rev.diffopt.close_file = 0;
rev.diffopt.no_free = 1;
} else {
int saved;

32
diff.c
View File

@ -6336,6 +6336,32 @@ static void diff_flush_patch_all_file_pairs(struct diff_options *o)
}
}
static void diff_free_file(struct diff_options *options)
{
if (options->close_file)
fclose(options->file);
}
static void diff_free_ignore_regex(struct diff_options *options)
{
int i;
for (i = 0; i < options->ignore_regex_nr; i++) {
regfree(options->ignore_regex[i]);
free(options->ignore_regex[i]);
}
free(options->ignore_regex);
}
void diff_free(struct diff_options *options)
{
if (options->no_free)
return;
diff_free_file(options);
diff_free_ignore_regex(options);
}
void diff_flush(struct diff_options *options)
{
struct diff_queue_struct *q = &diff_queued_diff;
@ -6399,8 +6425,7 @@ void diff_flush(struct diff_options *options)
* options->file to /dev/null should be safe, because we
* aren't supposed to produce any output anyway.
*/
if (options->close_file)
fclose(options->file);
diff_free_file(options);
options->file = xfopen("/dev/null", "w");
options->close_file = 1;
options->color_moved = 0;
@ -6433,8 +6458,7 @@ void diff_flush(struct diff_options *options)
free_queue:
free(q->queue);
DIFF_QUEUE_CLEAR(q);
if (options->close_file)
fclose(options->file);
diff_free(options);
/*
* Report the content-level differences with HAS_CHANGES;

15
diff.h
View File

@ -49,7 +49,17 @@
* - Once you finish feeding the pairs of files, call `diffcore_std()`.
* This will tell the diffcore library to go ahead and do its work.
*
* - Calling `diff_flush()` will produce the output.
* - Calling `diff_flush()` will produce the output, it will call
* `diff_free()` to free any resources, e.g. those allocated in
* `diff_opt_parse()`.
*
* - Set `.no_free = 1` before calling `diff_flush()` to defer the
* freeing of allocated memory in diff_options. This is useful when
* `diff_flush()` is being called in a loop, rather than as a
* one-off. When setting `.no_free = 1` you must ensure that
* `diff_free()` is called at the end, either by flipping the flag
* before the last `diff_flush()` call, or by flipping it before
* calling `diff_free()` yourself.
*/
struct combine_diff_path;
@ -365,6 +375,8 @@ struct diff_options {
struct repository *repo;
struct option *parseopts;
int no_free;
};
unsigned diff_filter_bit(char status);
@ -559,6 +571,7 @@ void diffcore_fix_diff_index(void);
int diff_queue_is_empty(void);
void diff_flush(struct diff_options*);
void diff_free(struct diff_options*);
void diff_warn_rename_limit(const char *varname, int needed, int degraded_cc);
/* diff-raw status letters */

View File

@ -963,12 +963,14 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
int log_tree_commit(struct rev_info *opt, struct commit *commit)
{
struct log_info log;
int shown, close_file = opt->diffopt.close_file;
int shown;
/* maybe called by e.g. cmd_log_walk(), maybe stand-alone */
int no_free = opt->diffopt.no_free;
log.commit = commit;
log.parent = NULL;
opt->loginfo = &log;
opt->diffopt.close_file = 0;
opt->diffopt.no_free = 1;
if (opt->line_level_traverse)
return line_log_print(opt, commit);
@ -985,7 +987,7 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)
fprintf(opt->diffopt.file, "\n%s\n", opt->break_bar);
opt->loginfo = NULL;
maybe_flush_or_die(opt->diffopt.file, "stdout");
if (close_file)
fclose(opt->diffopt.file);
opt->diffopt.no_free = no_free;
diff_free(&opt->diffopt);
return shown;
}