dir: make clear_directory() free all relevant memory

The calling convention for the dir API is supposed to end with a call to
clear_directory() to free up no longer needed memory.  However,
clear_directory() didn't free dir->entries or dir->ignored.  I believe
this was an oversight, but a number of callers noticed memory leaks and
started free'ing these.  Unfortunately, they did so somewhat haphazardly
(sometimes freeing the entries in the arrays, and sometimes only
free'ing the arrays themselves).  This suggests the callers weren't
trying to make sure any possible memory used might be free'd, but just
the memory they noticed their usecase definitely had allocated.

Fix this mess by moving all the duplicated free'ing logic into
clear_directory().  End by resetting dir to a pristine state so it could
be reused if desired.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/clean.c b/builtin/clean.c
index 5a9c29a558bb1efc383b41256972b3113845be4e..4ffe00dd7f3f9f9c7455054be4aceb61d6e7d68f 100644 (file)
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -1021,11 +1021,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
                string_list_append(&del_list, rel);
        }
 
-       for (i = 0; i < dir.nr; i++)
-               free(dir.entries[i]);
-
-       for (i = 0; i < dir.ignored_nr; i++)
-               free(dir.ignored[i]);
+       clear_directory(&dir);
 
        if (interactive && del_list.nr > 0)
                interactive_main_loop();

diff --git a/builtin/stash.c b/builtin/stash.c
index 10d87630cd0017f177b8c33929e5a94adaee6b0e..da48533d490f686c82fd85d9381cff22d0dd6d47 100644 (file)
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -875,11 +875,8 @@ static int get_untracked_files(const struct pathspec *ps, int include_untracked,
                strbuf_addstr(untracked_files, ent->name);
                /* NUL-terminate: will be fed to update-index -z */
                strbuf_addch(untracked_files, '\0');
-               free(ent);
        }
 
-       free(dir.entries);
-       free(dir.ignored);
        clear_directory(&dir);
        return found;
 }

diff --git a/dir.c b/dir.c
index fe64be30ed651de44f2aeb6c9527416e4af3da53..aa96030031fa801e2311b48e9f64055e92e779ae 100644 (file)
--- a/dir.c
+++ b/dir.c
@@ -3009,8 +3009,8 @@ int remove_path(const char *name)
 }
 
 /*
- * Frees memory within dir which was allocated for exclude lists and
- * the exclude_stack.  Does not free dir itself.
+ * Frees memory within dir which was allocated, and resets fields for further
+ * use.  Does not free dir itself.
  */
 void clear_directory(struct dir_struct *dir)
 {
@@ -3030,6 +3030,13 @@ void clear_directory(struct dir_struct *dir)
                free(group->pl);
        }
 
+       for (i = 0; i < dir->ignored_nr; i++)
+               free(dir->ignored[i]);
+       for (i = 0; i < dir->nr; i++)
+               free(dir->entries[i]);
+       free(dir->ignored);
+       free(dir->entries);
+
        stk = dir->exclude_stack;
        while (stk) {
                struct exclude_stack *prev = stk->prev;
@@ -3037,6 +3044,8 @@ void clear_directory(struct dir_struct *dir)
                stk = prev;
        }
        strbuf_release(&dir->basebuf);
+
+       memset(&dir, 0, sizeof(*dir));
 }
 
 struct ondisk_untracked_cache {

diff --git a/dir.h b/dir.h
index 5855c065a610b794b3777405bba6f343230b7ea2..7d76d0644f2ff811337df004ec8dca7358c896d1 100644 (file)
--- a/dir.h
+++ b/dir.h
@@ -36,7 +36,7 @@
  *
  * - Use `dir.entries[]`.
  *
- * - Call `clear_directory()` when none of the contained elements are no longer in use.
+ * - Call `clear_directory()` when the contained elements are no longer in use.
  *
  */
 

diff --git a/wt-status.c b/wt-status.c
index d75399085dead1ad52b36a12b505eef02b2036cc..c00ea3e06a395a49a07068c46dbbac6c50db9f9d 100644 (file)
--- a/wt-status.c
+++ b/wt-status.c
@@ -724,18 +724,14 @@ static void wt_status_collect_untracked(struct wt_status *s)
                struct dir_entry *ent = dir.entries[i];
                if (index_name_is_other(istate, ent->name, ent->len))
                        string_list_insert(&s->untracked, ent->name);
-               free(ent);
        }
 
        for (i = 0; i < dir.ignored_nr; i++) {
                struct dir_entry *ent = dir.ignored[i];
                if (index_name_is_other(istate, ent->name, ent->len))
                        string_list_insert(&s->ignored, ent->name);
-               free(ent);
        }
 
-       free(dir.entries);
-       free(dir.ignored);
        clear_directory(&dir);
 
        if (advice_status_u_option)