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

VFS improvements #93

Closed
kevinaboos opened this issue Jan 22, 2019 · 11 comments
Closed

VFS improvements #93

kevinaboos opened this issue Jan 22, 2019 · 11 comments

Comments

@kevinaboos
Copy link
Member

kevinaboos commented Jan 22, 2019

✔️ The ls app has redundant code that has been copy-pasted for two identical cases: when the path is specified, and when there is no path specified. Condense those into one case.

✔️ All file creation should accept a &DirRef rather than a WeakDirRef. This eliminates the potential error of not being able to upgrade the weak ref to the parent.

✔️ Don't use the return keyword at the end of a function, that violates Rust's style conventions.

✔️ TaskFiles are never actually created. Mmi files are pretty worthless at the moment.

@kevinaboos
Copy link
Member Author

We also need the ability to create symlinks (symbolic links) for both files and directories. These types should basically just be simple wrappers around FileRef and DirRef.

@kevinaboos
Copy link
Member Author

I don't understand parts of the Path crate, especially how canonicalize works. In my mind, canonicalize would have a similar fn signature as get(), in that it accepts just one directory, the starting/working directory from which to begin the path traversal.

Also, there are two types of canonicalization/verification for paths. One operates purely at the String level and does not access the filesystem. You can think of it as Path reduction/simplification. The other kind is "full canonicalization" that actually checks whether the files and directories really exist. A full canonicalization would then transform the Path's inner String into an absolute path.

@kevinaboos
Copy link
Member Author

kevinaboos commented Jan 23, 2019

✔️ a lot of functions don't need to borrow self mutably, e.g., get_child(). Fixed already.

@kevinaboos
Copy link
Member Author

kevinaboos commented Jan 23, 2019

✔️ your existing canonicalize() and relative() functions in the Path struct appended a slash instead of prepending it. I've fixed that.

@kevinaboos
Copy link
Member Author

kevinaboos commented Jan 23, 2019

✔️ The Path::get() function didn't allow you to get a File path, only a directory path. Fixed.

@kevinaboos
Copy link
Member Author

kevinaboos commented Jan 23, 2019

✔️ Absolute paths don't work, such as cd /root/namespaces when you're in some other directory.
(perhaps my changes caused this)

@kevinaboos
Copy link
Member Author

kevinaboos commented Jan 23, 2019

✔️ When adding nodes with something like insert_child(), we always must check if the child exists already. If it does, then we need to return an Err. You could optionally add an overwrite boolean parameter that if true, will result in the child node being replaced with the new one.
Note that when inserting a child, it should always be an error if you try to replace a file with a directory, or vice versa.

@kevinaboos
Copy link
Member Author

kevinaboos commented Jan 23, 2019

✔️ separate the creation of a file from the writing of its contents.

theseus-os pushed a commit that referenced this issue Jan 24, 2019
* Using filesystem to manage crate files. Removed kludgy usage of bootloader-provided ModuleAreas.

* Clarifies personality loading, a CrateNamespace can only load crate files from its directory.

* Partially addresses issue #93
@kevinaboos
Copy link
Member Author

kevinaboos commented Jan 30, 2019

✔️ Two other problems:

  • path traversal doesn't work for absolute paths... every path is treated as if it's relative, even when it begins with a /.
  • The root directory should just be /, not /root. There's no need to give it a name, that breaks expectations.

@kevinaboos
Copy link
Member Author

Thanks to @christinewang5, commit 5bc3fa1 addresses the problems of absolute path traversal and root directory naming.

@kevinaboos
Copy link
Member Author

Commit 9237398 covers all features, except for symlinks (see #114).

kevinaboos added a commit that referenced this issue Sep 1, 2020
* Using filesystem to manage crate files. Removed kludgy usage of bootloader-provided ModuleAreas.

* Clarifies personality loading, a CrateNamespace can only load crate files from its directory.

* Partially addresses issue #93
kevinaboos added a commit that referenced this issue Sep 1, 2020
* Using filesystem to manage crate files. Removed kludgy usage of bootloader-provided ModuleAreas.

* Clarifies personality loading, a CrateNamespace can only load crate files from its directory.

* Partially addresses issue #93
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

No branches or pull requests

3 participants