Skip to content

Commit dad4f23

Browse files
newrengitster
authored andcommitted
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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 2befe97 commit dad4f23

File tree

5 files changed

+13
-15
lines changed

5 files changed

+13
-15
lines changed

builtin/clean.c

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1021,11 +1021,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
10211021
string_list_append(&del_list, rel);
10221022
}
10231023

1024-
for (i = 0; i < dir.nr; i++)
1025-
free(dir.entries[i]);
1026-
1027-
for (i = 0; i < dir.ignored_nr; i++)
1028-
free(dir.ignored[i]);
1024+
clear_directory(&dir);
10291025

10301026
if (interactive && del_list.nr > 0)
10311027
interactive_main_loop();

builtin/stash.c

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -875,11 +875,8 @@ static int get_untracked_files(const struct pathspec *ps, int include_untracked,
875875
strbuf_addstr(untracked_files, ent->name);
876876
/* NUL-terminate: will be fed to update-index -z */
877877
strbuf_addch(untracked_files, '\0');
878-
free(ent);
879878
}
880879

881-
free(dir.entries);
882-
free(dir.ignored);
883880
clear_directory(&dir);
884881
return found;
885882
}

dir.c

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3009,8 +3009,8 @@ int remove_path(const char *name)
30093009
}
30103010

30113011
/*
3012-
* Frees memory within dir which was allocated for exclude lists and
3013-
* the exclude_stack. Does not free dir itself.
3012+
* Frees memory within dir which was allocated, and resets fields for further
3013+
* use. Does not free dir itself.
30143014
*/
30153015
void clear_directory(struct dir_struct *dir)
30163016
{
@@ -3030,13 +3030,22 @@ void clear_directory(struct dir_struct *dir)
30303030
free(group->pl);
30313031
}
30323032

3033+
for (i = 0; i < dir->ignored_nr; i++)
3034+
free(dir->ignored[i]);
3035+
for (i = 0; i < dir->nr; i++)
3036+
free(dir->entries[i]);
3037+
free(dir->ignored);
3038+
free(dir->entries);
3039+
30333040
stk = dir->exclude_stack;
30343041
while (stk) {
30353042
struct exclude_stack *prev = stk->prev;
30363043
free(stk);
30373044
stk = prev;
30383045
}
30393046
strbuf_release(&dir->basebuf);
3047+
3048+
memset(&dir, 0, sizeof(*dir));
30403049
}
30413050

30423051
struct ondisk_untracked_cache {

dir.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
*
3737
* - Use `dir.entries[]`.
3838
*
39-
* - Call `clear_directory()` when none of the contained elements are no longer in use.
39+
* - Call `clear_directory()` when the contained elements are no longer in use.
4040
*
4141
*/
4242

wt-status.c

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -724,18 +724,14 @@ static void wt_status_collect_untracked(struct wt_status *s)
724724
struct dir_entry *ent = dir.entries[i];
725725
if (index_name_is_other(istate, ent->name, ent->len))
726726
string_list_insert(&s->untracked, ent->name);
727-
free(ent);
728727
}
729728

730729
for (i = 0; i < dir.ignored_nr; i++) {
731730
struct dir_entry *ent = dir.ignored[i];
732731
if (index_name_is_other(istate, ent->name, ent->len))
733732
string_list_insert(&s->ignored, ent->name);
734-
free(ent);
735733
}
736734

737-
free(dir.entries);
738-
free(dir.ignored);
739735
clear_directory(&dir);
740736

741737
if (advice_status_u_option)

0 commit comments

Comments
 (0)