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 the 'memfs' example to pass the tests #17

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

denisovpi
Copy link
Contributor

Fix #16

Also, see discussion

@denisovpi denisovpi changed the title Fix the 'memfs' example to pass the tests [WIP] Fix the 'memfs' example to pass the tests Dec 29, 2024
@denisovpi denisovpi marked this pull request as draft December 29, 2024 02:59
@denisovpi denisovpi changed the title [WIP] Fix the 'memfs' example to pass the tests Fix the 'memfs' example to pass the tests Dec 29, 2024
@denisovpi denisovpi marked this pull request as ready for review December 29, 2024 19:58
dokan/examples/memfs/main.rs Show resolved Hide resolved
if create_options & FILE_DIRECTORY_FILE > 0 {
if let Some(stream_info) = &name.stream_info {
if !stream_info.check_default(true)? {
Err(STATUS_NOT_A_DIRECTORY)?;
Copy link
Member

Choose a reason for hiding this comment

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

Should this return ?

Copy link
Contributor Author

@denisovpi denisovpi Dec 31, 2024

Choose a reason for hiding this comment

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

Yes, we should return an error if the directory has streams (except for the default stream ':$I30:$INDEX_ALLOCATION'). I could insert this check elsewhere (for exp, line 532), but then it would have to be inserted in multiple places, for different operations (CreateDirectory, RemoveDirectory, etc)

Copy link
Member

Choose a reason for hiding this comment

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

Sorry I meant that I believe return is missing before Err(STATUS_NOT_A_DIRECTORY)?; ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I see. In Rust, the expressions return Err(...); and Err(...)?; are equivalent. I didn't see in the Rust guidelines which option is recommended to use. I've seen both usages in other rust projects. Overall you are right, I will change this to make the code more consistent.

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.

Open read-only file with FILE_FLAG_DELETE_ON_CLOSE flag
2 participants