Skip to content

dll: fuse: change name limit to 255 chars #474

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

Merged
merged 3 commits into from
Dec 8, 2022
Merged

dll: fuse: change name limit to 255 chars #474

merged 3 commits into from
Dec 8, 2022

Conversation

zeho11
Copy link
Contributor

@zeho11 zeho11 commented Dec 7, 2022

Fixes #191
Fixes #455


Before submitting this PR please review this checklist. Ideally all checkmarks should be checked upon submitting. (Use an x inside square brackets like so: [x])

  • Contributing: You MUST read and be willing to accept the CONTRIBUTOR AGREEMENT. The agreement gives joint copyright interests in your contributions to you and the original WinFsp author. If you have already accepted the CONTRIBUTOR AGREEMENT you do not need to do so again.
  • Topic branch: Avoid creating the PR off the master branch of your fork. Consider creating a topic branch and request a pull from that. This allows you to add commits to the master branch of your fork without affecting this PR.
  • No tabs: Consistently use SPACES everywhere. NO TABS, unless the file format requires it (e.g. Makefile).
  • Style: Follow the same code style as the rest of the project.
  • Tests: Include tests to the extent that it is possible, especially if you add a new feature.
  • Quality: Your design and code should be of high quality and something that you are proud of.

@billziss-gh
Copy link
Collaborator

billziss-gh commented Dec 7, 2022

Thank you for the PR. Your changes look good.

For me to accept this PR there are some additional requirements:

  • Resolve the (minor) changes I highlighted in my review.

  • Verify that your change does indeed fix the issue. The WinFsp testsuite includes the test create_namelen_dotest that checks path component length. Please append to this test checks for paths that contain a character such as : one check for a path with MaxComponentLength - 1 such characters that should succeed, one check for a path with MaxComponentLength such characters that should succeed, and one check for a path with MaxComponentLength + 1 such characters that should fail.

    • Your checks should go above this line.
    • You should use the character's code point and not embed the character in the source code directly, because Visual Studio has weird rules about which character set it uses for compilation.
    • If you want to debug your test changes set winfsp-tests as the startup project in Visual Studio and supply create_namelen_test as its command line argument. Place a breakpont on create_namelen_dotest and you are good to go.
    • You should verify that the updated test passes on NTFS. To do this use the command line option --external for winfsp-tests.
  • CI tests must pass. However if the updated create_namelen_test reveals a problem in a non-FUSE file system, then I will still accept your PR and fix the non-FUSE file system myself.

  • You MUST read and be willing to accept the CONTRIBUTOR AGREEMENT. The agreement gives joint copyright interests in your contributions to you and the original WinFsp author. If you have already accepted the CONTRIBUTOR AGREEMENT you do not need to do so again.

@billziss-gh
Copy link
Collaborator

Thank you for your work. Your changes look great. I am merging them in.

@billziss-gh billziss-gh merged commit 46054c0 into winfsp:master Dec 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

FUSE: Files with long unicode file name are not listed in Explorer FUSE: ensure path component limit is 255 chars (not bytes)
2 participants