From: Derrick Stolee Date: Thu, 30 Jan 2020 18:17:25 GMT Subject: Re: [PATCH 4/6] dir: move setting of nested_repo next to its actual usage Message-ID: <21d3d07a-d83d-a46c-ba5f-56c90326cb48@gmail.com> In-Reply-To: On 1/30/2020 11:20 AM, Elijah Newren wrote: > On Thu, Jan 30, 2020 at 8:10 AM Derrick Stolee 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!