Re: [PATCHv3 3/6] line-log.c: fix a memleak

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 31, 2015, 05:06 UTC
Message-ID: <xmqqsiclsqlz.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1427764931-27745-4-git-send-email-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

Show 16 quoted lines

> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  line-log.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/line-log.c b/line-log.c
> index b43ac58..db6e58d 100644
> --- a/line-log.c
> +++ b/line-log.c
> @@ -1129,6 +1129,9 @@ static int process_ranges_ordinary_commit(struct rev_info *rev, struct commit *c
>  	changed = process_all_files(&parent_range, rev, &queue, range);
>  	if (parent)
>  		add_line_range(rev, parent, parent_range);
> +	else
> +		free_line_log_data(parent_range);
> +

Should this be in the else clause? From a cursory scan add-line-range feeds it to either line-log-data-merge or line-log-data-copy to create "new" that is a separate thing from the parent_range we are feeding it, and that copy is used by add-decoration. If add-line-range does not take ownership of parent_range from us, shouldn't we be freeing it whether we called it or not???

>  	return changed;
>  }