Re: [PATCH] read-cache.c: free cache entry when refreshing fails

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

Stefan Beller <sbeller@google.com> writes:

> This fixes a memory leak, when building the cache entries as
> refresh_cache_entry may decide to return NULL in case of
>

in case of what?

I briefly wondered if refresh_cache_ent() should do the freeing but
that does not make sense at all if done unconditionally because the
other caller of the function does want to retain ce on error, and it
makes little sense to invent FREE_CE_ON_ERROR bit in refresh_option,
either, so this fix looks sensible.

> I am not sure how severe this memory leak is as it was found by
> scan.coverity.com, CID 290041.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  read-cache.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/read-cache.c b/read-cache.c
> index 9cff715..8d71860 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -725,7 +725,7 @@ struct cache_entry *make_cache_entry(unsigned int mode,
>  		unsigned int refresh_options)
>  {
>  	int size, len;
> -	struct cache_entry *ce;
> +	struct cache_entry *ce, *ret;
>  
>  	if (!verify_path(path)) {
>  		error("Invalid path '%s'", path);
> @@ -742,7 +742,13 @@ struct cache_entry *make_cache_entry(unsigned int mode,
>  	ce->ce_namelen = len;
>  	ce->ce_mode = create_ce_mode(mode);
>  
> -	return refresh_cache_entry(ce, refresh_options);
> +	ret = refresh_cache_entry(ce, refresh_options);
> +	if (!ret) {
> +		free(ce);
> +		return NULL;
> +	} else {
> +		return ret;
> +	}
>  }
>  
>  int ce_same_name(const struct cache_entry *a, const struct cache_entry *b)