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

WIP: Update fuse mount methods to take AsRef<OsStr> as options #131

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions examples/hello.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,6 @@ impl Filesystem for HelloFS {
fn main() {
env_logger::init();
let mountpoint = env::args_os().nth(1).unwrap();
let options = ["-o", "ro", "-o", "fsname=hello"]
.iter()
.map(|o| o.as_ref())
.collect::<Vec<&OsStr>>();
let options = OsStr::new("-o ro -o fsname=hello");
fuse::mount(HelloFS, mountpoint, &options).unwrap();
}
3 changes: 2 additions & 1 deletion examples/null.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,6 @@ impl Filesystem for NullFS {}
fn main() {
env_logger::init();
let mountpoint = env::args_os().nth(1).unwrap();
fuse::mount(NullFS, mountpoint, &[]).unwrap();

fuse::mount(NullFS, mountpoint, &"").unwrap();
}
17 changes: 9 additions & 8 deletions src/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@ use crate::reply::ReplySender;

/// Helper function to provide options as a fuse_args struct
/// (which contains an argc count and an argv pointer)
fn with_fuse_args<T, F: FnOnce(&fuse_args) -> T>(options: &[&OsStr], f: F) -> T {
let mut args = vec![CString::new("rust-fuse").unwrap()];
args.extend(options.iter().map(|s| CString::new(s.as_bytes()).unwrap()));
fn with_fuse_args<T, F: FnOnce(&fuse_args) -> T>(options: &OsStr, f: F) -> T {
let args = vec![
CString::new("rust-fuse").unwrap(),
CString::new(options.to_str().unwrap()).unwrap()
];
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a change in behavior: the current code passes FUSE a list of N arguments; this code always gives it only 2 arguments, where everything after the first is combined into one string. This doesn't actually work correctly; if you play with the example a bit, you'll notice that everything past the first space character gets ignored by FUSE.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the feedback. Ill mark the PR as WIP while I take a look at this.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 a good way to test this is by passing --help as one of the arguments. libfuse will print its usage text if it sees that.

let argptrs: Vec<_> = args.iter().map(|s| s.as_ptr()).collect();
f(&fuse_args { argc: argptrs.len() as i32, argv: argptrs.as_ptr(), allocated: 0 })
}
Expand All @@ -33,7 +35,7 @@ impl Channel {
/// given path. The kernel driver will delegate filesystem operations of
/// the given path to the channel. If the channel is dropped, the path is
/// unmounted.
pub fn new(mountpoint: &Path, options: &[&OsStr]) -> io::Result<Channel> {
pub fn new(mountpoint: &Path, options: &OsStr) -> io::Result<Channel> {
let mountpoint = mountpoint.canonicalize()?;
with_fuse_args(options, |args| {
let mnt = CString::new(mountpoint.as_os_str().as_bytes())?;
Expand Down Expand Up @@ -164,11 +166,10 @@ mod test {

#[test]
fn fuse_args() {
with_fuse_args(&[OsStr::new("foo"), OsStr::new("bar")], |args| {
assert_eq!(args.argc, 3);
with_fuse_args(&OsStr::new("foo bar"), |args| {
assert_eq!(args.argc, 2);
assert_eq!(unsafe { CStr::from_ptr(*args.argv.offset(0)).to_bytes() }, b"rust-fuse");
assert_eq!(unsafe { CStr::from_ptr(*args.argv.offset(1)).to_bytes() }, b"foo");
assert_eq!(unsafe { CStr::from_ptr(*args.argv.offset(2)).to_bytes() }, b"bar");
assert_eq!(unsafe { CStr::from_ptr(*args.argv.offset(1)).to_bytes() }, b"foo bar");
});
}
}
8 changes: 4 additions & 4 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -367,15 +367,15 @@ pub trait Filesystem {
///
/// Note that you need to lead each option with a separate `"-o"` string. See
/// `examples/hello.rs`.
pub fn mount<FS: Filesystem, P: AsRef<Path>>(filesystem: FS, mountpoint: P, options: &[&OsStr]) -> io::Result<()>{
Session::new(filesystem, mountpoint.as_ref(), options).and_then(|mut se| se.run())
pub fn mount<FS: Filesystem, P: AsRef<Path>, O: AsRef<OsStr>>(filesystem: FS, mountpoint: P, options: O) -> io::Result<()> {
Session::new(filesystem, mountpoint.as_ref(), options.as_ref()).and_then(|mut se| se.run())
}

/// Mount the given filesystem to the given mountpoint. This function spawns
/// a background thread to handle filesystem operations while being mounted
/// and therefore returns immediately. The returned handle should be stored
/// to reference the mounted filesystem. If it's dropped, the filesystem will
/// be unmounted.
pub unsafe fn spawn_mount<'a, FS: Filesystem+Send+'a, P: AsRef<Path>>(filesystem: FS, mountpoint: P, options: &[&OsStr]) -> io::Result<BackgroundSession<'a>> {
Session::new(filesystem, mountpoint.as_ref(), options).and_then(|se| se.spawn())
pub unsafe fn spawn_mount<'a, FS: Filesystem+Send+'a, P: AsRef<Path>, O: AsRef<OsStr>>(filesystem: FS, mountpoint: P, options: O) -> io::Result<BackgroundSession<'a>> {
Session::new(filesystem, mountpoint.as_ref(), options.as_ref()).and_then(|se| se.spawn())
}
2 changes: 1 addition & 1 deletion src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ pub struct Session<FS: Filesystem> {

impl<FS: Filesystem> Session<FS> {
/// Create a new session by mounting the given filesystem to the given mountpoint
pub fn new(filesystem: FS, mountpoint: &Path, options: &[&OsStr]) -> io::Result<Session<FS>> {
pub fn new(filesystem: FS, mountpoint: &Path, options: &OsStr) -> io::Result<Session<FS>> {
info!("Mounting {}", mountpoint.display());
Channel::new(mountpoint, options).map(|ch| {
Session {
Expand Down