8000 Use std::filesystem::weakly_canonical() to avoid filesyste_error by t-sin · Pull Request #62 · mimium-org/mimium · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
This repository was archived by the owner on Dec 22, 2024. It is now read-only.

Use std::filesystem::weakly_canonical() to avoid filesyste_error #62

Merged
merged 1 commit into from
Mar 9, 2021
Merged

Use std::filesystem::weakly_canonical() to avoid filesyste_error #62

merged 1 commit into from
Mar 9, 2021

Conversation

t-sin
Copy link
Contributor
@t-sin t-sin commented Mar 9, 2021

This PR fixes a bug about path normalization.

Bug behavior

This bug occurs when the users specify absolute path but it actually does not exist. When mimium command is passed such a path, the command fails like this:

$ $ mimium /path/does/not/exist/foo.mmm
filesystem error: cannot make canonical path: No such file or directory [../../../../path/does/not/exist/foo.mmm]

Additionally, if a path specified exists, mimium command normalizes the path and then continues its process.

I found this behavior in development of mimium mapckage manager.

The cause of this bug

mimium does path normalization at first of its process but it has not done correctly.

The path normalization has done at this point:

mimium/src/frontend/genericapp.cpp

Line 64 in 263f357

compiler.setFilePath(input ? fs::absolute(input.value().filepath).string() : "/stdin");

According to this page, the function std::filesystem::canonical() throws filesystem_error when the path does not exist. That page also says that the function does fully existence check for all path elements.

How to fix

That page also says std::filesystem::weakly_canonical() does the path normalization partially. It means that the specified path so the path entirely may not exist.

If using weakly_canonical() instead of canonical(), the mimium command successfully normalizes at the point of the source code, checks input path existence then handles by the situation by itself, like this:

$ cmake --build ./build/ -j && ./build/src/mimium /path/does/not/exist/foo.mmm
File not found: /path/does/not/exist/foo.mmm

So, in this PR's case, using weakly_canonical() is better.

tomoyanonymous
tomoyanonymous approved these changes Mar 9, 2021
View reviewed changes
Copy link
Contributor
@tomoyanonymous tomoyanonymous left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks.

@tomoyanonymous
Copy link
Contributor

@all-contributors please add @t-sin for code

@allcontributors allcontributors bot mentioned this pull request Mar 9, 2021
Merged
@allcontributors
Copy link
Contributor

@tomoyanonymous

I've put up a pull request to add @t-sin! 🎉

@tomoyanonymous tomoyanonymous merged commit c0e6255 into dev Mar 9, 2021
@tomoyanonymous tomoyanonymous mentioned this pull request Mar 9, 2021
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@tomoyanonymous tomoyanonymous tomoyanonymous approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@t-sin @tomoyanonymous
0