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

File.cpp: revert get_parent_dir change #14086

Merged
merged 2 commits into from
Jun 24, 2023
Merged

Conversation

elad335
Copy link
Contributor

@elad335 elad335 commented Jun 24, 2023

Should fix #14031 .
Tagging @brian218 to explain the change.
Issues's problem is that std::path("$locks") returns "$locks/" with japanese locale.
Keep in mind that we already call setlocal("C") on startup so some code overwrites it.

Some places in code also use the pattern (file_path.substr(get_parent_dir(file2_path_in_x_directory, x_times).size()) == trail path from parent directory) which breaks with the code on master.

@brian218
Copy link
Contributor

brian218 commented Jun 24, 2023

The changes introduced in 6a570ae simplified the implementation of the function get_parent_dir() and made it neater while keeping the same aim.
In additional, the original implementation (as well as the one brought back by this PR) seems to be unable to handle or normalize some badly-composed paths.
See the test below:
Image

Test code:

for (int i = 0; i < 7; i++)
{
    std::string test = "/dev_hdd0//./AA//./BB/..///CC///DD/.//EE/FF//////";
    test = get_parent_dir(test, i);
    std::cout << test << std::endl;
}

@brian218
Copy link
Contributor

Also, since the sys_fs stuff now keeps track of the the mount paths passed to sys_fs_mount(), it's really important to properly normalize each paths so that the paths can be correctly compared or looked up in the sys_fs map.

@elad335
Copy link
Contributor Author

elad335 commented Jun 24, 2023

If you want to organize badly origanized paths you may add another function then, it's not this function's purpose.

@brian218
Copy link
Contributor

brian218 commented Jun 24, 2023

If you want to orginize badly composed paths you may add another function then, it's not this function's purpose.

But it was even not properly getting the parent dir of some given paths. 😆

@elad335
Copy link
Contributor Author

elad335 commented Jun 24, 2023

Examples or it didn't happen.

@brian218
Copy link
Contributor

Examples or it didn't happen.

Image

As you can see, /BB/.. was never correctly handled in this case.

@elad335
Copy link
Contributor Author

elad335 commented Jun 24, 2023

It's not a bug, both paths mean the same thing, with the difference being filesystems typically also check for BB's existence in case it's symlinking to another path.

@Nekotekina
Copy link
Member

I think path normalization should be a separate function

Utilities/File.h Outdated Show resolved Hide resolved
string view version of the argument path, use with care.
@Nekotekina Nekotekina merged commit 09f83e4 into RPCS3:master Jun 24, 2023
4 of 5 checks passed
@elad335 elad335 deleted the fs-hotfix branch May 6, 2024 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Regression] RPCS3 fails to launch with Japanese OS locale
3 participants