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>

diff --git a/builtin/rm.c b/builtin/rm.c
index 3a0e0eaab7d1fd8a298bb2519776abede5c685e2..171f37c1cc5371c307ef16ce67fefc1c40fbc2c2 100644 (file)
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -311,7 +311,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
                if (!match_pathspec_depth(&pathspec, ce->name, ce_namelen(ce), 0, seen))
                        continue;
                ALLOC_GROW(list.entry, list.nr + 1, list.alloc);
                if (!match_pathspec_depth(&pathspec, ce->name, ce_namelen(ce), 0, seen))
                        continue;
                ALLOC_GROW(list.entry, list.nr + 1, list.alloc);

-               list.entry[list.nr].name = ce->name;
+               list.entry[list.nr].name = xstrdup(ce->name);

                list.entry[list.nr].is_submodule = S_ISGITLINK(ce->ce_mode);
                if (list.entry[list.nr++].is_submodule &&
                    !is_staging_gitmodules_ok())
                list.entry[list.nr].is_submodule = S_ISGITLINK(ce->ce_mode);
                if (list.entry[list.nr++].is_submodule &&
                    !is_staging_gitmodules_ok())

diff --git a/builtin/update-index.c b/builtin/update-index.c
index c8f0d5f47ce36df8bd13afca3cee8c185bfa7743..00313f373aadd989e0627b0b3068676c66c17a9b 100644 (file)
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -559,6 +559,7 @@ static int do_reupdate(int ac, const char **av,
                const struct cache_entry *ce = active_cache[pos];
                struct cache_entry *old = NULL;
                int save_nr;
                const struct cache_entry *ce = active_cache[pos];
                struct cache_entry *old = NULL;
                int save_nr;

+               char *path;

 
                if (ce_stage(ce) || !ce_path_match(ce, &pathspec))
                        continue;
 
                if (ce_stage(ce) || !ce_path_match(ce, &pathspec))
                        continue;


@@ -575,7 +576,9 @@ static int do_reupdate(int ac, const char **av,
                 * or worse yet 'allow_replace', active_nr may decrease.
                 */
                save_nr = active_nr;
                 * or worse yet 'allow_replace', active_nr may decrease.
                 */
                save_nr = active_nr;

-               update_one(ce->name);
+               path = xstrdup(ce->name);
+               update_one(path);
+               free(path);

                if (save_nr != active_nr)
                        goto redo;
        }
                if (save_nr != active_nr)
                        goto redo;
        }

diff --git a/read-cache.c b/read-cache.c
index 00af9addd90d9b44f9134d50708548896deb0e2f..3f735f3c8e5dd0a2cdfc89c41bab363bda21bcc6 100644 (file)
--- a/read-cache.c
+++ b/read-cache.c
@@ -47,6 +47,7 @@ static void replace_index_entry(struct index_state *istate, int nr, struct cache
        struct cache_entry *old = istate->cache[nr];
 
        remove_name_hash(istate, old);
        struct cache_entry *old = istate->cache[nr];
 
        remove_name_hash(istate, old);

+       free(old);

        set_index_entry(istate, nr, ce);
        istate->cache_changed = 1;
 }
        set_index_entry(istate, nr, ce);
        istate->cache_changed = 1;
 }


@@ -478,6 +479,7 @@ int remove_index_entry_at(struct index_state *istate, int pos)
 
        record_resolve_undo(istate, ce);
        remove_name_hash(istate, ce);
 
        record_resolve_undo(istate, ce);
        remove_name_hash(istate, ce);

+       free(ce);

        istate->cache_changed = 1;
        istate->cache_nr--;
        if (pos >= istate->cache_nr)
        istate->cache_changed = 1;
        istate->cache_nr--;
        if (pos >= istate->cache_nr)


@@ -499,8 +501,10 @@ void remove_marked_cache_entries(struct index_state *istate)
        unsigned int i, j;
 
        for (i = j = 0; i < istate->cache_nr; i++) {
        unsigned int i, j;
 
        for (i = j = 0; i < istate->cache_nr; i++) {

-               if (ce_array[i]->ce_flags & CE_REMOVE)
+               if (ce_array[i]->ce_flags & CE_REMOVE) {

                        remove_name_hash(istate, ce_array[i]);
                        remove_name_hash(istate, ce_array[i]);

+                       free(ce_array[i]);
+               }

                else
                        ce_array[j++] = ce_array[i];
        }
                else
                        ce_array[j++] = ce_array[i];
        }


@@ -1894,7 +1898,7 @@ int read_index_unmerged(struct index_state *istate)
                new_ce->ce_mode = ce->ce_mode;
                if (add_index_entry(istate, new_ce, 0))
                        return error("%s: cannot drop to stage #0",
                new_ce->ce_mode = ce->ce_mode;
                if (add_index_entry(istate, new_ce, 0))
                        return error("%s: cannot drop to stage #0",

-                                    ce->name);
+                                    new_ce->name);

                i = index_name_pos(istate, new_ce->name, len);
        }
        return unmerged;
                i = index_name_pos(istate, new_ce->name, len);
        }
        return unmerged;

diff --git a/resolve-undo.c b/resolve-undo.c
index c09b00664e6892c84424bb4984152883460d3df6..49ebaaf8d8b269b374ad3ccbfc57acccd83fe006 100644 (file)
--- a/resolve-undo.c
+++ b/resolve-undo.c
@@ -119,6 +119,7 @@ int unmerge_index_entry_at(struct index_state *istate, int pos)
        struct string_list_item *item;
        struct resolve_undo_info *ru;
        int i, err = 0, matched;
        struct string_list_item *item;
        struct resolve_undo_info *ru;
        int i, err = 0, matched;

+       char *name;

 
        if (!istate->resolve_undo)
                return pos;
 
        if (!istate->resolve_undo)
                return pos;


@@ -138,20 +139,22 @@ int unmerge_index_entry_at(struct index_state *istate, int pos)
        if (!ru)
                return pos;
        matched = ce->ce_flags & CE_MATCHED;
        if (!ru)
                return pos;
        matched = ce->ce_flags & CE_MATCHED;

+       name = xstrdup(ce->name);

        remove_index_entry_at(istate, pos);
        for (i = 0; i < 3; i++) {
                struct cache_entry *nce;
                if (!ru->mode[i])
                        continue;
                nce = make_cache_entry(ru->mode[i], ru->sha1[i],
        remove_index_entry_at(istate, pos);
        for (i = 0; i < 3; i++) {
                struct cache_entry *nce;
                if (!ru->mode[i])
                        continue;
                nce = make_cache_entry(ru->mode[i], ru->sha1[i],

-                                      ce->name, i + 1, 0);
+                                      name, i + 1, 0);

                if (matched)
                        nce->ce_flags |= CE_MATCHED;
                if (add_index_entry(istate, nce, ADD_CACHE_OK_TO_ADD)) {
                        err = 1;
                if (matched)
                        nce->ce_flags |= CE_MATCHED;
                if (add_index_entry(istate, nce, ADD_CACHE_OK_TO_ADD)) {
                        err = 1;

-                       error("cannot unmerge '%s'", ce->name);
+                       error("cannot unmerge '%s'", name);

                }
        }
                }
        }

+       free(name);

        if (err)
                return pos;
        free(ru);
        if (err)
                return pos;
        free(ru);