diff options
author | Elijah Newren <newren@gmail.com> | 2019-10-01 11:55:24 -0700 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2019-10-02 12:06:58 +0900 |
commit | 69f272b922df153c86db520bf9b6018a9808c2a6 (patch) | |
tree | 16a3beef56937e0060c10890220027ff26a1e680 /builtin/diff-files.c | |
parent | clean: fix theoretical path corruption (diff) | |
download | tgif-69f272b922df153c86db520bf9b6018a9808c2a6.tar.xz |
dir: special case check for the possibility that pathspec is NULL
Commits 404ebceda01c ("dir: also check directories for matching
pathspecs", 2019-09-17) and 89a1f4aaf765 ("dir: if our pathspec might
match files under a dir, recurse into it", 2019-09-17) added calls to
match_pathspec() and do_match_pathspec() passing along their pathspec
parameter. Both match_pathspec() and do_match_pathspec() assume the
pathspec argument they are given is non-NULL. It turns out that
unpack-tree.c's verify_clean_subdirectory() calls read_directory() with
pathspec == NULL, and it is possible on case insensitive filesystems for
that NULL to make it to these new calls to match_pathspec() and
do_match_pathspec(). Add appropriate checks on the NULLness of pathspec
to avoid a segfault.
In case the negation throws anyone off (one of the calls was to
do_match_pathspec() while the other was to !match_pathspec(), yet no
negation of the NULLness of pathspec is used), there are two ways to
understand the differences:
* The code already handled the pathspec == NULL cases before this
series, and this series only tried to change behavior when there was
a pathspec, thus we only want to go into the if-block if pathspec is
non-NULL.
* One of the calls is for whether to recurse into a subdirectory, the
other is for after we've recursed into it for whether we want to
remove the subdirectory itself (i.e. the subdirectory didn't match
but something under it could have). That difference in situation
leads to the slight differences in logic used (well, that and the
slightly unusual fact that we don't want empty pathspecs to remove
untracked directories by default).
Denton found and analyzed one issue and provided the patch for the
match_pathspec() call, SZEDER figured out why the issue only reproduced
for some folks and not others and provided the testcase, and I looked
through the remainder of the series and noted the do_match_pathspec()
call that should have the same check.
Co-authored-by: Denton Liu <liu.denton@gmail.com>
Co-authored-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to 'builtin/diff-files.c')
0 files changed, 0 insertions, 0 deletions