[PATCH v2] 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>
---
Note: Applies on top of en/clean-nested-with-ignored, in next.

As with v1, the authorship is really mixed, so I don't know if I should use Co-authored-by (highlighted as a possibility by Denton), or the far more common Helped-by (as suggested by Junio but based on a more limited summary of the different contributions), or if perhaps Denton or SZEDER should be marked as the author and I be marked as Helped-by or Co-authored-by. Since Denton commented on round 1, I used his suggestion for attribution in this round, but I'm open to changing it to whatever works best.

Changes since v1:
  - Removed comments that made sense in context of the original thread
    but wouldn't be helpful to future readers.
  - s/Helped-by/Co-authored-by/

Range-diff:
1:  885c22d24b ! 1:  c495b9303c dir: special case check for the possibility that pathspec is NULL
    @@ t/t0050-filesystem.sh: $test_unicode 'merge (silent unicode normalization)' '
     +
     +		git checkout --orphan todo &&
     +		git reset --hard &&
    -+		# the subdir is crucial, without it there is no segfault
     +		mkdir -p gitweb/subdir &&
     +		>gitweb/subdir/file &&
     +		# it is not strictly necessary to add and commit the
    -+		# gitweb directory, its presence is sufficient
     +		git add gitweb &&
     +		git commit -m "add gitweb/subdir/file" &&
     +

 dir.c                 |  8 +++++---
 t/t0050-filesystem.sh | 21 +++++++++++++++++++++
 2 files changed, 26 insertions(+), 3 deletions(-)