FIXUP: Address review comments

jmmv · Oct 31, 2018 · 99e76b5 · 99e76b5
1 parent 49e7700
commit 99e76b5
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 25 deletions.
diff --git a/src/lib.rs b/src/lib.rs
@@ -441,15 +441,15 @@ impl fuse::Filesystem for SandboxFS {
             return;
         }
 
-        let nix_mode = mode.map(|m| sys::stat::Mode::from_bits_truncate(m as sys::stat::mode_t));
-        if mode.is_some() {
-            let mode = mode.unwrap() as sys::stat::mode_t;
-            let nix_mode = nix_mode.expect("Must be present if mode is present").bits();
-            if mode != nix_mode {
-                warn!("setattr on inode {} with mode {} can only apply mode {}",
-                    inode, mode, nix_mode);
+        let nix_mode = mode.map(|m| {
+            let sys_mode = m as sys::stat::mode_t;
+            let nix_mode = sys::stat::Mode::from_bits_truncate(sys_mode);
+            if sys_mode != nix_mode.bits() {
+                warn!("setattr on inode {} with mode {} can only apply mode {:?}",
+                    inode, sys_mode, nix_mode);
             }
-        }
+            nix_mode
+        });
 
         let values = nodes::AttrDelta {
             mode: nix_mode,

diff --git a/src/nodes/conv.rs b/src/nodes/conv.rs
@@ -52,9 +52,11 @@ fn system_time_to_timespec(path: &Path, name: &str, time: &io::Result<SystemTime
 }
 
 /// Converts a `time::Timespec` object into a `sys::time::TimeVal`.
+// TODO(jmmv): Consider upstreaming this function or a constructor for TimeVal that takes the two
+// components separately.
 pub fn timespec_to_timeval(spec: Timespec) -> sys::time::TimeVal {
-    const S_TO_NS: i64 = 1000 * 1000 * 1000;
-    sys::time::TimeVal::nanoseconds(spec.sec * S_TO_NS + i64::from(spec.nsec))
+    use sys::time::TimeVal;
+    TimeVal::seconds(spec.sec) + TimeVal::nanoseconds(spec.nsec.into())
 }
 
 /// Converts a file type as returned by the file system to a FUSE file type.

diff --git a/src/nodes/dir.rs b/src/nodes/dir.rs
@@ -250,21 +250,15 @@ impl Dir {
 
     /// Same as `getattr` but with the node already locked.
     fn getattr_unlocked(inode: u64, state: &mut MutableDir) -> NodeResult<fuse::FileAttr> {
-        let new_attr = match state.underlying_path.as_ref() {
-            Some(path) => {
-                let fs_attr = fs::symlink_metadata(path)?;
-                if !fs_attr.is_dir() {
-                    warn!("Path {:?} backing a directory node is no longer a directory; got {:?}",
-                        path, fs_attr.file_type());
-                    return Err(KernelError::from_errno(errno::Errno::EIO));
-                }
-                Some(conv::attr_fs_to_fuse(path, inode, &fs_attr))
-            },
-            None => None,
+        if let Some(path) = &state.underlying_path {
+            let fs_attr = fs::symlink_metadata(path)?;
+            if !fs_attr.is_dir() {
+                warn!("Path {:?} backing a directory node is no longer a directory; got {:?}",
+                    path, fs_attr.file_type());
+                return Err(KernelError::from_errno(errno::Errno::EIO));
+            }
+            state.attr = conv::attr_fs_to_fuse(path, inode, &fs_attr);
         };
-        if let Some(new_attr) = new_attr {
-            state.attr = new_attr;
-        }
 
         Ok(state.attr)
     }
@@ -370,7 +364,7 @@ impl Node for Dir {
 
     fn setattr(&self, delta: &AttrDelta) -> NodeResult<fuse::FileAttr> {
         let mut state = self.state.lock().unwrap();
-        if let Some(path) = state.underlying_path.as_ref() {
+        if let Some(path) = &state.underlying_path {
             setattr(path, &state.attr, delta)?;
         }
         Dir::getattr_unlocked(self.inode, &mut state)