Re: [PATCH] dir.c: avoid stat() in valid_cached_dir()

From: Duy Nguyen <pclouds@gmail.com>
Date: Jan 1, 2018, 23:57 UTC
Message-ID: <CACsJy8BnxOz9brnkyZ58guTsUhgKKN_XQvbYaZJz17888pgHoQ@mail.gmail.com>
In-Reply-To: <xmqq608qllcd.fsf@gitster.mtv.corp.google.com>

On Fri, Dec 29, 2017 at 2:10 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 23 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
>
>>>  [1] https://public-inbox.org/git/CACsJy8AmbKSp0mDLRaDCWn45veeNc03B-Gq8r8PQPrDt9bM_EA@mail.gmail.com/
>>>
>>>  dir.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/dir.c b/dir.c
>>> index 7c4b45e30e..edcb7bb462 100644
>>> --- a/dir.c
>>> +++ b/dir.c
>>> @@ -1809,7 +1809,7 @@ static int valid_cached_dir(struct dir_struct *dir,
>>>       */
>>>      refresh_fsmonitor(istate);
>>>      if (!(dir->untracked->use_fsmonitor && untracked->valid)) {
>>> -            if (stat(path->len ? path->buf : ".", &st)) {
>>> +            if (lstat(path->len ? path->buf : ".", &st)) {
>
> Hmph, I have to wonder if this is sufficient, though.  When you got
> "a/b/c" in path->buf, is it sufficient to avoid mistaking 'c' that
> is (eh, rather, "has unexpectedly turned into") a symbolic link to a
> directory as a true directory?  Wouldn't we have to be equally careful
> about "a" and "a/b" as well?

We do. But the way directory traversal works, I believe a and a/b must have been checked (by this function too) before we call valid_cached_dir("a/b/c").

-- 
Duy