On 1/30/2020 11:20 AM, Elijah Newren wrote:
> On Thu, Jan 30, 2020 at 8:10 AM Derrick Stolee <stolee@xxxxxxxxx> wrote:
>> diff --git a/dir.c b/dir.c
>> index b460211e61..0989558ae6 100644
>> --- a/dir.c
>> +++ b/dir.c
>> @@ -1660,29 +1660,26 @@ static enum path_treatment treat_directory(struct dir_struct *dir,
>> const struct pathspec *pathspec)
>> {
>> int nested_repo = 0;
>> -
>> /* The "len-1" is to strip the final '/' */
>> - switch (directory_exists_in_index(istate, dirname, len-1)) {
>> - case index_directory:
>> - return path_recurse;
>> + enum exist_status status = directory_exists_in_index(istate, dirname, len-1);
>>
>> - case index_gitdir:
>> + if (status == index_directory)
>> + return path_recurse;
>> + if (status == index_gitdir)
>> return path_none;
>
> I think right here we should add:
>
> if (status != index_nonexistent):
> BUG("Unhandled value for directory_exists_in_index:
> %d\n", status);
>
> for future-proofing, since both you and I had to look up what
> possibilities existed as a return status from
> directory_exists_in_index(), and I'd rather a large warning was thrown
> if someone ever adds a fourth option to that function rather than
> assume treat_directory() is fine and only needs to special case two
> choices.
>
> Or we could add an assert or a code comment, just so long as we
> document to future readers that the remainder of the code is assuming
> status==index_nonexistent.
I'm happy if you squash this into the commit. Thanks!
