Re: [PATCH v3 12/22] builtin/fast-export: fix leaking diff options

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

> Before calling `handle_commit()` in a loop, we set `diffopt.no_free`
> such that its contents aren't getting freed inside of `handle_commit()`.
> We never unset that flag though, which means that it'll ultimately leak
> when calling `release_revisions()`.
>
> Fix this by unsetting the flag after the loop.

If I grep for 

    $ git grep -nH -E -e '(\.|->)no_free' \*.c

I notice that in a lot of places there is a pattern of doing

    set .no_free to 1
    cause a bunch of diff using the same set of options
    set .no_free to 0
    call diff_free().

I am curious why we do not need any diff_free() here?

> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/fast-export.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/builtin/fast-export.c b/builtin/fast-export.c
> index 4b6e8c6832..fe92d2436c 100644
> --- a/builtin/fast-export.c
> +++ b/builtin/fast-export.c
> @@ -1278,9 +1278,11 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
>  	revs.diffopt.format_callback = show_filemodify;
>  	revs.diffopt.format_callback_data = &paths_of_changed_objects;
>  	revs.diffopt.flags.recursive = 1;
> +
>  	revs.diffopt.no_free = 1;
>  	while ((commit = get_revision(&revs)))
>  		handle_commit(commit, &revs, &paths_of_changed_objects);
> +	revs.diffopt.no_free = 0;
>  
>  	handle_tags_and_duplicates(&extra_refs);
>  	handle_tags_and_duplicates(&tag_refs);