read-cache.c: fix memory leaks caused by removed cache entries

When cache_entry structs are removed from index_state.cache, they are not properly freed. Freeing those entries wasn't possible before because we couldn't remove them from index_state.name_hash. Now that we _do_ remove the entries from name_hash, we can also free them. Add 'free(cache_entry)' to all call sites of name-hash.c::remove_name_hash in read-cache.c (we could free() directly in remove_name_hash(), but name-hash.c isn't concerned with cache_entry allocation at all). Accessing a cache_entry after removing it from the index is now no longer allowed, as the memory has been freed. The following functions need minor fixes (typically by copying ce->name before use): - builtin/rm.c::cmd_rm - builtin/update-index.c::do_reupdate - read-cache.c::read_index_unmerged - resolve-undo.c::unmerge_index_entry_at Signed-off-by: Karsten Blees <blees@dcon.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Karsten Blees committed Nov 14, 2013 at 20:24 UTC 5699d17ee0949e6c01311a03dcfce485fcdd9b1a
4 files changed +16 -6
builtin/rm.c
+1 -1
@@ -311,7 +311,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
311 if (!match_pathspec_depth(&pathspec, ce->name, ce_namelen(ce), 0, seen))
312 continue;
313 ALLOC_GROW(list.entry, list.nr + 1, list.alloc);
314 - list.entry[list.nr].name = ce->name;
314 + list.entry[list.nr].name = xstrdup(ce->name);
315 list.entry[list.nr].is_submodule = S_ISGITLINK(ce->ce_mode);
316 if (list.entry[list.nr++].is_submodule &&
317 !is_staging_gitmodules_ok())
builtin/update-index.c
+4 -1
@@ -559,6 +559,7 @@ static int do_reupdate(int ac, const char **av,
559 const struct cache_entry *ce = active_cache[pos];
560 struct cache_entry *old = NULL;
561 int save_nr;
562 + char *path;
563
564 if (ce_stage(ce) || !ce_path_match(ce, &pathspec))
565 continue;
@@ -575,7 +576,9 @@ static int do_reupdate(int ac, const char **av,
576 * or worse yet 'allow_replace', active_nr may decrease.
577 */
578 save_nr = active_nr;
578 - update_one(ce->name);
579 + path = xstrdup(ce->name);
580 + update_one(path);
581 + free(path);
582 if (save_nr != active_nr)
583 goto redo;
584 }
read-cache.c
+6 -2
@@ -47,6 +47,7 @@ static void replace_index_entry(struct index_state *istate, int nr, struct cache
47 struct cache_entry *old = istate->cache[nr];
48
49 remove_name_hash(istate, old);
50 + free(old);
51 set_index_entry(istate, nr, ce);
52 istate->cache_changed = 1;
53 }
@@ -478,6 +479,7 @@ int remove_index_entry_at(struct index_state *istate, int pos)
479
480 record_resolve_undo(istate, ce);
481 remove_name_hash(istate, ce);
482 + free(ce);
483 istate->cache_changed = 1;
484 istate->cache_nr--;
485 if (pos >= istate->cache_nr)
@@ -499,8 +501,10 @@ void remove_marked_cache_entries(struct index_state *istate)
501 unsigned int i, j;
502
503 for (i = j = 0; i < istate->cache_nr; i++) {
502 - if (ce_array[i]->ce_flags & CE_REMOVE)
504 + if (ce_array[i]->ce_flags & CE_REMOVE) {
505 remove_name_hash(istate, ce_array[i]);
506 + free(ce_array[i]);
507 + }
508 else
509 ce_array[j++] = ce_array[i];
510 }
@@ -1894,7 +1898,7 @@ int read_index_unmerged(struct index_state *istate)
1898 new_ce->ce_mode = ce->ce_mode;
1899 if (add_index_entry(istate, new_ce, 0))
1900 return error("%s: cannot drop to stage #0",
1897 - ce->name);
1901 + new_ce->name);
1902 i = index_name_pos(istate, new_ce->name, len);
1903 }
1904 return unmerged;
resolve-undo.c
+5 -2
@@ -119,6 +119,7 @@ int unmerge_index_entry_at(struct index_state *istate, int pos)
119 struct string_list_item *item;
120 struct resolve_undo_info *ru;
121 int i, err = 0, matched;
122 + char *name;
123
124 if (!istate->resolve_undo)
125 return pos;
@@ -138,20 +139,22 @@ int unmerge_index_entry_at(struct index_state *istate, int pos)
139 if (!ru)
140 return pos;
141 matched = ce->ce_flags & CE_MATCHED;
142 + name = xstrdup(ce->name);
143 remove_index_entry_at(istate, pos);
144 for (i = 0; i < 3; i++) {
145 struct cache_entry *nce;
146 if (!ru->mode[i])
147 continue;
148 nce = make_cache_entry(ru->mode[i], ru->sha1[i],
147 - ce->name, i + 1, 0);
149 + name, i + 1, 0);
150 if (matched)
151 nce->ce_flags |= CE_MATCHED;
152 if (add_index_entry(istate, nce, ADD_CACHE_OK_TO_ADD)) {
153 err = 1;
152 - error("cannot unmerge '%s'", ce->name);
154 + error("cannot unmerge '%s'", name);
155 }
156 }
157 + free(name);
158 if (err)
159 return pos;
160 free(ru);