Jeff King <peff@xxxxxxxx> writes:
> While looping over the index entries, when we see a higher level stage
> the first thing we do is allocate a combine_diff_path struct for it. But
> this can leak; if check_removed() returns an error, we'll continue to
> the next iteration of the loop without cleaning up.
>
> We can fix this by just delaying the allocation by a few lines.
>
> I don't think this leak is triggered in the test suite, but it's pretty
> easy to see by inspection. My ulterior motive here is that the delayed
> allocation means we have all of the data needed to initialize "dpath" at
> the time of malloc, making it easier to factor out a constructor
> function.
>
> Signed-off-by: Jeff King <peff@xxxxxxxx>
> ---
> diff-lib.c | 24 ++++++++++++------------
> 1 file changed, 12 insertions(+), 12 deletions(-)
Makes sense.
>
> diff --git a/diff-lib.c b/diff-lib.c
> index c6d3bc4d37..85b8f1fa59 100644
> --- a/diff-lib.c
> +++ b/diff-lib.c
> @@ -156,18 +156,6 @@ void run_diff_files(struct rev_info *revs, unsigned int option)
> size_t path_len;
> struct stat st;
>
> - path_len = ce_namelen(ce);
> -
> - dpath = xmalloc(combine_diff_path_size(5, path_len));
> - dpath->path = (char *) &(dpath->parent[5]);
> -
> - dpath->next = NULL;
> - memcpy(dpath->path, ce->name, path_len);
> - dpath->path[path_len] = '\0';
> - oidclr(&dpath->oid, the_repository->hash_algo);
> - memset(&(dpath->parent[0]), 0,
> - sizeof(struct combine_diff_parent)*5);
> -
> changed = check_removed(ce, &st);
> if (!changed)
> wt_mode = ce_mode_from_stat(ce, st.st_mode);
> @@ -178,7 +166,19 @@ void run_diff_files(struct rev_info *revs, unsigned int option)
> }
> wt_mode = 0;
> }
> +
> + path_len = ce_namelen(ce);
> +
> + dpath = xmalloc(combine_diff_path_size(5, path_len));
> + dpath->path = (char *) &(dpath->parent[5]);
> +
> + dpath->next = NULL;
> + memcpy(dpath->path, ce->name, path_len);
> + dpath->path[path_len] = '\0';
> + oidclr(&dpath->oid, the_repository->hash_algo);
> dpath->mode = wt_mode;
> + memset(&(dpath->parent[0]), 0,
> + sizeof(struct combine_diff_parent)*5);
>
> while (i < entries) {
> struct cache_entry *nce = istate->cache[i];
