Refactor and combine all FileType structs in yanix#945
Merged
Conversation
This commit does a bit of everything: refactors bits here and there, fixes a bug discovered in another bytecodealliance#701, and combines all structs that we used in `yanix` and `wasi-common` crates to represent file types on *nix into one struct, `yanix::file::FileType`. Up until now, in `yanix`, we've had two separate structs used to represent file types on the host: `yanix::dir::FileType` and `yanix::file::SFlags` (well, not quite, but that was its main use). They both were used in different context (the former when parsing `dirent` struct, and the latter when parsing `stat` struct), they were C-compatible (as far as their representation goes), and as it turns out, they shared possible enumeration values. This commit combines them both into an idiomatic Rust enum with the caveat that it is now *not* C-compatible, however, I couldn't find a single use where that would actually matter, and even if it does in the future, we can simply add appropriate impl methods. The combine `yanix::file::FileType` struct can be constructed in two ways: 1) either from `stat.st_mode` value (and while we're here, now it's done correctly according to POSIX which fixes the bug mentioned in VFS impl PR bytecodealliance#701), or 2) from `dirent.d_type` value. Also, since we now have one struct for representing both contexts, this cleans up nicely a lot of duplicated code in `host` module.
Merged
Member
Author
|
I'm gonna go ahead and merge this to not thwart progress in #701, but if anything is amiss, please feel free to reopen, revert, or file an issue. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This commit does a bit of everything: refactors bits here and there,
fixes a bug discovered in another #701, and combines all structs that
we used in
yanixandwasi-commoncrates to represent file typeson *nix into one struct,
yanix::file::FileType.Up until now, in
yanix, we've had two separate structs used torepresent file types on the host:
yanix::dir::FileTypeandyanix::file::SFlags(well, not quite, but that was its main use).They both were used in different context (the former when parsing
direntstruct, and the latter when parsingstatstruct), theywere C-compatible (as far as their representation goes), and as it
turns out, they shared possible enumeration values. This commit
combines them both into an idiomatic Rust enum with the caveat that
it is now not C-compatible, however, I couldn't find a single use
where that would actually matter, and even if it does in the future,
we can simply add appropriate impl methods.
The combine
yanix::file::FileTypestruct can be constructed in twoways: 1) either from
stat.st_modevalue (and while we're here,now it's done correctly according to POSIX which fixes the bug mentioned
in VFS impl PR #701), or 2) from
dirent.d_typevalue. Also, since we nowhave one struct for representing both contexts, this cleans up nicely
a lot of duplicated code in
hostmodule.cc @iximeow Hopefully, this fixes the bug you mentioned in #701!