Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix file_exists relative path bug #7932

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

huzhiguang
Copy link
Contributor

a.txt real location is a/a.txt

file_exists("a/b/../a.txt");

this code return value is false ,but hhvm process relative path,so this result is true,diff php,and then old way process relative path performance have problem,this modify way performance is improve 40%

replay case:

#3148

@@ -193,7 +193,15 @@ static int accessSyscall(
return -1;
}
}
return ::access(File::TranslatePathWithFileCache(path).data(), mode);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this change will disable the StaticFileCache feature.

The cause of this bug is that FileUtil::canonicalize is used in File::TranslatePathWithFileCache and File::TranslatePathKeepRelative. So what's the purpose of FileUtil::canonicalize? It a path is canonicalized, the access() result may be different.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think FileCache has no meaning, please see the code (File: TranslatePathWithFileCache),the logic is :
!translated.empty() && access(translated.data(), F_OK) < 0 && StaticContentCache::TheFileCache

so the physical file does not exist, and then the file cache won't exist.
I don't think this code is meaningful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants