Re: [PATCH] read-cache.c: free cache entry when refreshing fails
From: Junio C Hamano <hidden>
Date: 2016-06-15 23:03:50
Possibly related (same subject, not in this thread)
- 2016-06-15 · [PATCH] read-cache.c: free cache entry when refreshing fails · Stefan Beller <hidden>
Stefan Beller [off-list ref] 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.
quoted hunk
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 <redacted> --- 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)