From 2d9426b049335c1a39be7ea7416094e944bfe63c Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Fri, 20 Mar 2015 17:28:00 -0700 Subject: read-cache: free cache entry in add_to_index in case of early return This frees `ce` would be leaking in the error path. Additionally a free is moved towards the return. This helps code readability as we often have this pattern of freeing resources just before return/exit and not in between the code. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- read-cache.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'read-cache.c') diff --git a/read-cache.c b/read-cache.c index 8d71860f69..60abec6055 100644 --- a/read-cache.c +++ b/read-cache.c @@ -681,15 +681,18 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st, alias = index_file_exists(istate, ce->name, ce_namelen(ce), ignore_case); if (alias && !ce_stage(alias) && !ie_match_stat(istate, alias, st, ce_option)) { /* Nothing changed, really */ - free(ce); if (!S_ISGITLINK(alias->ce_mode)) ce_mark_uptodate(alias); alias->ce_flags |= CE_ADDED; + + free(ce); return 0; } if (!intent_only) { - if (index_path(ce->sha1, path, st, HASH_WRITE_OBJECT)) + if (index_path(ce->sha1, path, st, HASH_WRITE_OBJECT)) { + free(ce); return error("unable to index file %s", path); + } } else set_object_name_for_intent_to_add_entry(ce); -- cgit v1.2.3 From 067178ed8a7822e6bc88ad606b707fc33658e6fc Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 23 Mar 2015 10:58:00 -0700 Subject: add_to_index(): free unused cache-entry We allocate a cache-entry pretty early in the function and then decide either not to do anything when we are pretending to add, or add it and then get an error (another possibility is obviously to succeed). When pretending or failing to add, we forgot to free the cache-entry. Noticed during a discussion on Stefan's patch to change the coding style without fixing the issue ;-) Signed-off-by: Junio C Hamano --- read-cache.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'read-cache.c') diff --git a/read-cache.c b/read-cache.c index 60abec6055..5b922fd583 100644 --- a/read-cache.c +++ b/read-cache.c @@ -707,9 +707,11 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st, ce->ce_mode == alias->ce_mode); if (pretend) - ; - else if (add_index_entry(istate, ce, add_option)) - return error("unable to add %s to index",path); + free(ce); + else if (add_index_entry(istate, ce, add_option)) { + free(ce); + return error("unable to add %s to index", path); + } if (verbose && !was_same) printf("add '%s'\n", path); return 0; -- cgit v1.2.3 From 915e44c6357f3bd9d5fa498a201872c4367302d3 Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Mon, 23 Mar 2015 10:57:11 -0700 Subject: read-cache: fix memleak `ce` is allocated in make_cache_entry and should be freed if it is not used any more. refresh_cache_entry as a wrapper around refresh_cache_ent will either return - the `ce` given as the parameter, when it was up-to-date; - a new updated cache entry which is allocated to new memory; or - a NULL when refreshing failed. In the latter two cases, the original cache-entry `ce` is not used and needs to be freed. The rule can be expressed as "if the return value from refresh is different from the original ce, ce is no longer used." Helped-by: Junio C Hamano Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- read-cache.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'read-cache.c') diff --git a/read-cache.c b/read-cache.c index 5b922fd583..0052b72d9c 100644 --- a/read-cache.c +++ b/read-cache.c @@ -748,12 +748,9 @@ struct cache_entry *make_cache_entry(unsigned int mode, ce->ce_mode = create_ce_mode(mode); ret = refresh_cache_entry(ce, refresh_options); - if (!ret) { + if (ret != ce) free(ce); - return NULL; - } else { - return ret; - } + return ret; } int ce_same_name(const struct cache_entry *a, const struct cache_entry *b) -- cgit v1.2.3