Patrick Steinhardt <ps@xxxxxx> writes:
> In `treat_directory()` we perform some logic to handle ignored and
> untracked entries. When populating a directory with entries we first
> save the current number of ignored/untracked entries and then populate
> new entries at the end of our arrays that keep track of those entries.
> When we figure out that all entries have been ignored/are untracked we
> then remove this tail of entries from those vectors again. But there is
> an off by one error in both paths that causes us to not free the first
> ignored and untracked entries, respectively.
>
> Fix these off-by-one errors to plug the resulting leak. While at it,
> massage the code a bit to match our modern code style.
>
> Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
> ---
> dir.c | 6 ++----
> t/t3011-common-prefixes-and-directory-traversal.sh | 1 +
> t/t7061-wtstatus-ignore.sh | 1 +
> t/t7521-ignored-mode.sh | 1 +
> 4 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/dir.c b/dir.c
> index 5a23376bdae..787bcb7a1a4 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -2135,8 +2135,7 @@ static enum path_treatment treat_directory(struct dir_struct *dir,
> */
> state = path_none;
> } else {
> - int i;
> - for (i = old_ignored_nr + 1; i<dir->ignored_nr; ++i)
> + for (int i = old_ignored_nr; i < dir->ignored_nr; i++)
> FREE_AND_NULL(dir->ignored[i]);
> dir->ignored_nr = old_ignored_nr;
So the only effect this bug had was that the entry in the array at
the old_ignored_nr did not get freed, even though the code correctly
updated that the length of the dir->ignored[] after truncation. I
speculated, before looking at the code change but only from what was
in the proposed log message, that the resulting array ended up to
have an extra entry, or to lose an entry, due to miscounting, but
that is not what is going on. If the bug's nature was that we
screwed up the length of the updated array, somebody would certainly
have noticed a bug in the end-user observable behaviour that a path
that should be ignored is not getting ignored or the other way
around. Interesting.
I would imagine this is something hard to spot with code inspection
alone, yet fairly easy for the leak checker to notice. I am happy
to see that it does really help to have good code coverage in the
test suite.
Thanks.
