Re: [PATCH 1/2] diff.[ch]: have diff_free() call clear_pathspec(opts.pathspec)

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 16, 2022, 21:48 UTC
Message-ID: <xmqqh78ycwjc.fsf@gitster.g>
In-Reply-To: <patch-1.2-1c6c7f0f52f-20220216T105250Z-avarab@gmail.com>

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

Show 12 quoted lines

> diff --git a/diff.c b/diff.c
> index c862771a589..0aef3db6e10 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -6345,6 +6345,7 @@ void diff_free(struct diff_options *options)
>  
>  	diff_free_file(options);
>  	diff_free_ignore_regex(options);
> +	clear_pathspec(&options->pathspec);
>  }
>  
>  void diff_flush(struct diff_options *options)

Interesting. As diff_flush() is the way to conclude the diff session whose state was kept in the diff_options structure, it probably makes sense to allow pathspec to be also cleared from there. It is somewhat surprising that we didn't do this when we introduced diff_free(), but better late than never ;-)

Show 20 quoted lines

> diff --git a/notes-merge.c b/notes-merge.c
> index b4a3a903e86..7ba40cfb080 100644
> --- a/notes-merge.c
> +++ b/notes-merge.c
> @@ -175,7 +175,6 @@ static struct notes_merge_pair *diff_tree_remote(struct notes_merge_options *o,
>  		       oid_to_hex(&mp->remote));
>  	}
>  	diff_flush(&opt);
> -	clear_pathspec(&opt.pathspec);
>  
>  	*num_changes = len;
>  	return changes;
> @@ -261,7 +260,6 @@ static void diff_tree_local(struct notes_merge_options *o,
>  		       oid_to_hex(&mp->local));
>  	}
>  	diff_flush(&opt);
> -	clear_pathspec(&opt.pathspec);
>  }
>  
>  static void check_notes_merge_worktree(struct notes_merge_options *o)

Looks quite sensible.  Will queue.  Thanks.