@matheusbsbs / matheus-bagliano / commits / dad4f23ce5

Fork 0

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>

Elijah Newren committed Aug 18, 2020 at 22:58 UTC dad4f23ce59339bb32ad9e1cc1682c696f7a724f

5 files changed +13 -15

builtin/clean.c

+1 -5

@@ -1021,11 +1021,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)

1021 string_list_append(&del_list, rel);

1022 }

1023

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);

1025

1026 if (interactive && del_list.nr > 0)

1027 interactive_main_loop();

builtin/stash.c

-3

@@ -875,11 +875,8 @@ static int get_untracked_files(const struct pathspec *ps, int include_untracked,

875 strbuf_addstr(untracked_files, ent->name);

876 /* NUL-terminate: will be fed to update-index -z */

877 strbuf_addch(untracked_files, '\0');

878 - free(ent);

878 }

879

881 - free(dir.entries);

882 - free(dir.ignored);

880 clear_directory(&dir);

881 return found;

882 }

dir.c

+11 -2

@@ -3009,8 +3009,8 @@ int remove_path(const char *name)

3009 }

3010

3011 /*

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.

3014 */

3015 void clear_directory(struct dir_struct *dir)

3016 {

@@ -3030,6 +3030,13 @@ void clear_directory(struct dir_struct *dir)

3030 free(group->pl);

3031 }

3032

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 +

3040 stk = dir->exclude_stack;

3041 while (stk) {

3042 struct exclude_stack *prev = stk->prev;

@@ -3037,6 +3044,8 @@ void clear_directory(struct dir_struct *dir)

3044 stk = prev;

3045 }

3046 strbuf_release(&dir->basebuf);

3047 +

3048 + memset(&dir, 0, sizeof(*dir));

3049 }

3050

3051 struct ondisk_untracked_cache {

dir.h

+1 -1

@@ -36,7 +36,7 @@

36 *

37 * - Use `dir.entries[]`.

38 *

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.

40 *

41 */

wt-status.c

-4

@@ -724,18 +724,14 @@ static void wt_status_collect_untracked(struct wt_status *s)

724 struct dir_entry *ent = dir.entries[i];

725 if (index_name_is_other(istate, ent->name, ent->len))

726 string_list_insert(&s->untracked, ent->name);

727 - free(ent);

727 }

728

729 for (i = 0; i < dir.ignored_nr; i++) {

730 struct dir_entry *ent = dir.ignored[i];

731 if (index_name_is_other(istate, ent->name, ent->len))

732 string_list_insert(&s->ignored, ent->name);

734 - free(ent);

733 }

734

737 - free(dir.entries);

738 - free(dir.ignored);

735 clear_directory(&dir);

736

737 if (advice_status_u_option)