Add partial implementation of mount #46
Conversation
Codecov Report
@@ Coverage Diff @@
## master #46 +/- ##
==========================================
- Coverage 19.95% 19.01% -0.94%
==========================================
Files 46 47 +1
Lines 4680 5027 +347
Branches 1081 1183 +102
==========================================
+ Hits 934 956 +22
- Misses 3355 3663 +308
- Partials 391 408 +17
Continue to review full report at Codecov.
|
|
Can you refactor your code according to the dummy utility here? #47 |
Cargo.toml
Outdated
| # FIXME: this is only like this because we can't rename dependencies and we use the same macro in | ||
| # the tests that libmesabox uses to build | ||
| tar_util = ["libmesabox/tar_util"] | ||
| lsb = [ | ||
| "tar_util" | ||
| "mount", "tar_util" |
There was a problem hiding this comment.
Please split into multiple lines.
libmesabox/Cargo.toml
Outdated
| # XXX: temporary until renaming dependencies is supported | ||
| tar_util = ["tar", "globset"] | ||
| lsb = [ | ||
| "tar" | ||
| "mount", "tar" |
There was a problem hiding this comment.
Please split into multiple lines.
| extern crate clap; | ||
| extern crate lazy_static; | ||
| extern crate libc; | ||
| extern crate nix; |
There was a problem hiding this comment.
Please separate extern crate and use statements by an empty line.
libmesabox/src/lsb/mount/mod.rs
Outdated
| } | ||
|
|
||
| impl Uuid { | ||
| fn new() -> MountResult<Self> { |
There was a problem hiding this comment.
The code for this and Label (and maybe more that I haven't looked at yet) is almost exactly the same. With how minor the differences are, the common code should be split out into a separate function.
There was a problem hiding this comment.
Now I combined Label and Uuid together
libmesabox/src/lsb/mount/mod.rs
Outdated
| Ok(Self { path_map: path_map }) | ||
| } | ||
|
|
||
| fn get_device_path(&self, input: &OsString) -> MountResult<&Path> { |
There was a problem hiding this comment.
Same issue. This is almost exactly the same as for Label.
libmesabox/src/lsb/mount/mod.rs
Outdated
| fn new(path: &str) -> MountResult<Self> { | ||
| // all of these files should exist and can be read, but just in case | ||
| let file = fs::File::open(path).or(Err(MountError::OpenFileError(String::from(path))))?; | ||
| let mut entries = Vec::new(); |
There was a problem hiding this comment.
Use vec![] instead of Vec::new().
libmesabox/src/lsb/mount/mod.rs
Outdated
| let file = fs::File::open(path).or(Err(MountError::OpenFileError(String::from(path))))?; | ||
| let mut entries = Vec::new(); | ||
| for line in BufReader::new(file).lines() { | ||
| let line = line.or(Err(MountError::FSDescFileFormatError(String::from(path))))?; |
There was a problem hiding this comment.
I don't think these files are guaranteed to be valid UTF-8, so I think using Strings here is wrong.
There was a problem hiding this comment.
You are right. If these files contain invalid UTF-8, the program will return an error. Now I use read_until to read them in bytes.
libmesabox/src/lsb/mount/mod.rs
Outdated
| } | ||
| Some(_) => {} | ||
| } | ||
| let a: Vec<&str> = line.split_whitespace().collect(); |
There was a problem hiding this comment.
Rather than constructing a Vec, you can just use the iterator from line.split_whitespace() to get the data and if it returns None at some point than clearly you have less than six (6) columns. You can just do .count() after getting the data and see if there are exactly two (2) left columns.
libmesabox/src/lsb/mount/mod.rs
Outdated
| Ok(Self { entries: entries }) | ||
| } | ||
|
|
||
| fn get_output(&self, fs_type: &OsString) -> String { |
There was a problem hiding this comment.
Again, String seems wrong because the filesystem directory doesn't need to be valid UTF-8.
There was a problem hiding this comment.
The returned string is used for output, and it is generated by to_string_lossy. I guess this is the only way to get a printable OsString.
libmesabox/src/lsb/mount/mod.rs
Outdated
|
|
||
| impl MountOptions { | ||
| fn from_matches(matches: &clap::ArgMatches) -> MountResult<Self> { | ||
| let mut mount_list: Vec<Box<Mountable + Send>> = Vec::new(); |
There was a problem hiding this comment.
I would recommend trying to find another way to store the data, as this setup requires dynamic dispatch. If you can't, I suppose it's fine for this utility as this part doesn't need to be super fast.
There was a problem hiding this comment.
Now I defined an enum to do the dispatching.
Most LSB features are supported, except
-v(print diagnostic messages),-n(mount without writing in /etc/mtab), and-s(ignore mount options not supported by a file system type). Also, user mountable devices are not supported yet.Some tests need root privilege, they are marked as
#[ignore]. To run them, use the following command:sudo cargo test root_test_mount -- --ignored --test-threads 1. (We have to disable parallel tests, because the tests will mount the same device.)