Fixed a bug where attributes could not be retrieved for directories.

crates/btlib/src/blocktree.rs

@@ -17,7 +17,8 @@ mod private {
         io::{self, SeekFrom, Write},
         path::{Path, PathBuf},
         sync::{
-            atomic::{AtomicU64, Ordering}, RwLock,
+            atomic::{AtomicU64, Ordering},
+            RwLock,
         },
         time::Duration,
     };
@@ -92,7 +93,7 @@ mod private {
             match whence {
                 libc::SEEK_SET => Ok(SeekFrom::Start(offset)),
                 libc::SEEK_CUR => Ok(SeekFrom::Current(offset as i64)),
-                libc::SEEK_END => Ok(SeekFrom::End(-(offset as i64))),
+                libc::SEEK_END => Ok(SeekFrom::End(offset as i64)),
                 _ => Err(io::Error::new(
                     io::ErrorKind::InvalidInput,
                     "`whence` was not one of `libc::{SEEK_SET, SEEK_CUR, SEEK_END}`",
@@ -183,63 +184,72 @@ mod private {
         },
         Directory {
             dir: Directory,
-        }
+            block: RwLock<Box<dyn Block>>,
+        },
     }
 
     impl HandleValue {
-        fn new_file(block: Box<dyn Block>) -> HandleValue {
+        fn new(block: Box<dyn Block>) -> HandleValue {
             HandleValue::File {
                 block: RwLock::new(block),
             }
         }
 
-        fn new_dir(dir: Directory) -> HandleValue {
-            HandleValue::Directory { dir }
+        fn convert_to_dir(self) -> io::Result<HandleValue> {
+            let lock = self.take_block();
+            let dir = {
+                let mut guard = lock.write().err_to_string()?;
+                guard.seek(SeekFrom::Start(0))?;
+                read_from(&mut *guard)?
+            };
+            Ok(HandleValue::Directory { dir, block: lock })
         }
 
-        fn not_file_err<T>() -> io::Result<T> {
-            Err(io::Error::new(io::ErrorKind::Other, "handle is not for a file"))
+        fn take_block(self) -> RwLock<Box<dyn Block>> {
+            match self {
+                Self::File { block, .. } => block,
+                Self::Directory { block, .. } => block,
+            }
         }
 
-        fn block_mut(&mut self) -> io::Result<&mut Box<dyn Block>> {
+        fn block(&self) -> &RwLock<Box<dyn Block>> {
             match self {
-                Self::File { block, .. } => block
-                    .get_mut()
-                    .map_err(|err| io::Error::new(io::ErrorKind::Other, err.to_string())),
-                _ => Self::not_file_err(),
+                Self::File { block, .. } => block,
+                Self::Directory { block, .. } => block,
             }
         }
 
+        fn block_mut(&mut self) -> io::Result<&mut Box<dyn Block>> {
+            let lock = match self {
+                Self::File { block, .. } => block,
+                Self::Directory { block, .. } => block,
+            };
+            lock.get_mut().err_to_string()
+        }
+
         fn access_block<T, F: FnOnce(&Box<dyn Block>) -> io::Result<T>>(
             &self,
             cb: F,
         ) -> io::Result<T> {
-            match self {
-                Self::File { block, .. } => {
-                    let guard = block.read().err_to_string()?;
-                    cb(&guard)
-                }
-                _ => Self::not_file_err(),
-            }
+            let guard = self.block().read().err_to_string()?;
+            cb(&guard)
         }
 
         fn access_block_mut<T, F: FnOnce(&mut Box<dyn Block>) -> io::Result<T>>(
             &self,
             cb: F,
         ) -> io::Result<T> {
-            match self {
-                Self::File { block, .. } => {
-                    let mut guard = block.write().err_to_string()?;
-                    cb(&mut guard)
-                }
-                _ => Self::not_file_err(),
-            }
+            let mut guard = self.block().write().err_to_string()?;
+            cb(&mut guard)
         }
 
         fn directory(&self) -> io::Result<&Directory> {
             match self {
                 Self::Directory { dir, .. } => Ok(dir),
-                _ => Err(io::Error::new(io::ErrorKind::Other, "handle is not for a directory"))
+                _ => Err(io::Error::new(
+                    io::ErrorKind::Other,
+                    "handle is not for a directory",
+                )),
             }
         }
     }
@@ -259,7 +269,7 @@ mod private {
         fn new(block: Box<dyn Block>) -> InodeTableValue {
             const FIRST_HANDLE: Handle = 1;
             let mut handles = HashMap::with_capacity(1);
-            handles.insert(FIRST_HANDLE, HandleValue::new_file(block));
+            handles.insert(FIRST_HANDLE, HandleValue::new(block));
             Self {
                 handle_values: handles,
                 next_handle: FIRST_HANDLE + 1,
@@ -268,15 +278,34 @@ mod private {
             }
         }
 
-        fn invalid_handle_err(handle: Handle) -> Error {
-            Error::custom(format!("invalid handle {handle}"))
+        fn invalid_handle_err(handle: Handle) -> io::Error {
+            io::Error::new(io::ErrorKind::Other, format!("invalid handle {handle}"))
         }
 
-        fn block_mut(&mut self, handle: Handle) -> io::Result<&mut Box<dyn Block>> {
+        fn value(&self, handle: Handle) -> io::Result<&HandleValue> {
+            self.handle_values
+                .get(&handle)
+                .ok_or_else(|| Self::invalid_handle_err(handle))
+        }
+
+        fn value_mut(&mut self, handle: Handle) -> io::Result<&mut HandleValue> {
             self.handle_values
                 .get_mut(&handle)
-                .ok_or_else(|| Self::invalid_handle_err(handle))?
-                .block_mut()
+                .ok_or_else(|| Self::invalid_handle_err(handle))
+        }
+
+        fn block_mut(&mut self, handle: Handle) -> io::Result<&mut Box<dyn Block>> {
+            self.value_mut(handle)?.block_mut()
+        }
+
+        fn convert_to_dir(&mut self, handle: Handle) -> io::Result<()> {
+            let value = self
+                .handle_values
+                .remove(&handle)
+                .ok_or_else(|| Self::invalid_handle_err(handle))?;
+            let value = value.convert_to_dir()?;
+            self.handle_values.insert(handle, value);
+            Ok(())
         }
 
         fn access_block_mut<T, F: FnOnce(&mut Box<dyn Block>) -> io::Result<T>>(
@@ -284,10 +313,7 @@ mod private {
             handle: Handle,
             cb: F,
         ) -> io::Result<T> {
-            self.handle_values
-                .get(&handle)
-                .ok_or_else(|| Self::invalid_handle_err(handle))?
-                .access_block_mut(cb)
+            self.value(handle)?.access_block_mut(cb)
         }
 
         fn try_borrow_block<T, F: FnOnce(&mut Box<dyn Block>) -> io::Result<T>>(
@@ -298,36 +324,28 @@ mod private {
                 .unclaimed_handles
                 .last()
                 .ok_or_else(|| Error::custom("no handles available"))?;
-            let handle_value = self.handle_values.get_mut(handle).ok_or_else(|| {
-                let err = Error::custom(
-                    "logic error: can't find block associated with handle",
-                );
-                error!("{err}");
-                err
-            })?;
-            let block = handle_value.block_mut()?;
+            let value = self.value_mut(*handle)?;
+            let block = value.block_mut()?;
             cb(block)
         }
 
         fn insert(&mut self, block: Box<dyn Block>) {
             let handle = self.next_handle;
             self.next_handle += 1;
-            self.handle_values
-                .insert(handle, HandleValue::new_file(block));
+            self.handle_values.insert(handle, HandleValue::new(block));
             self.unclaimed_handles.push(handle);
         }
 
-        fn insert_then_get_mut(
-            &mut self,
-            block: Box<dyn Block>,
-        ) -> &mut Box<dyn Block> {
+        fn insert_then_get_mut(&mut self, block: Box<dyn Block>) -> &mut Box<dyn Block> {
             self.insert(block);
             let handle = self.unclaimed_handles.last().unwrap();
             self.block_mut(*handle).unwrap()
         }
 
-        fn take_handle(&mut self) -> Option<Handle> {
-            self.unclaimed_handles.pop()
+        fn take_handle(&mut self) -> io::Result<Handle> {
+            self.unclaimed_handles
+                .pop()
+                .ok_or_else(|| io::Error::new(io::ErrorKind::Other, "no unclaimed handles"))
         }
 
         fn give_handle(&mut self, handle: Handle) {
@@ -466,8 +484,14 @@ mod private {
             authorizer: A,
         ) -> Result<Blocktree<C, A>> {
             let mut inodes = HashMap::with_capacity(1);
-            inodes.insert(SpecInodes::Sb.into(), RwLock::new(InodeTableValue::new(sb_block)));
-            inodes.insert(SpecInodes::RootDir.into(), RwLock::new(InodeTableValue::new(root_block)));
+            inodes.insert(
+                SpecInodes::Sb.into(),
+                RwLock::new(InodeTableValue::new(sb_block)),
+            );
+            inodes.insert(
+                SpecInodes::RootDir.into(),
+                RwLock::new(InodeTableValue::new(root_block)),
+            );
             Ok(Blocktree {
                 path: btdir,
                 inodes: RwLock::new(inodes),
@@ -546,10 +570,7 @@ mod private {
             inode: Inode,
             cb: F,
         ) -> io::Result<T> {
-            let mut inodes = self
-                .inodes
-                .write()
-                .err_to_string()?;
+            let mut inodes = self.inodes.write().err_to_string()?;
             let entry = inodes.entry(inode);
             cb(entry)
         }
@@ -559,12 +580,13 @@ mod private {
             inode: Inode,
             cb: F,
         ) -> io::Result<T> {
-            let inodes = self
-                .inodes
+            let inodes = self.inodes.read().err_to_string()?;
+            let guard = inodes
+                .get(&inode)
+                .ok_or_else(|| Error::NotOpen(inode))?
                 .read()
                 .err_to_string()?;
-            let value = inodes.get(&inode).ok_or_else(|| Error::NotOpen(inode))?.read().err_to_string()?;
-            cb(&value)
+            cb(&guard)
         }
 
         fn access_value_mut<T, F: FnOnce(&mut InodeTableValue) -> io::Result<T>>(
@@ -572,12 +594,13 @@ mod private {
             inode: Inode,
             cb: F,
         ) -> io::Result<T> {
-            let inodes = self
-                .inodes
-                .read()
+            let inodes = self.inodes.read().err_to_string()?;
+            let mut guard = inodes
+                .get(&inode)
+                .ok_or_else(|| Error::NotOpen(inode))?
+                .write()
                 .err_to_string()?;
-            let mut value = inodes.get(&inode).ok_or_else(|| Error::NotOpen(inode))?.write().err_to_string()?;
-            cb(&mut value)
+            cb(&mut guard)
         }
 
         fn access_block_mut<T, F: FnOnce(&mut Box<dyn Block>) -> io::Result<T>>(
@@ -630,7 +653,7 @@ mod private {
             })
         }
 
-        fn take_block_if_ok<T, F: FnOnce(Handle, &mut Box<dyn Block>) -> io::Result<T>>(
+        fn take_handle_if_ok<T, F: FnOnce(Handle, &mut Box<dyn Block>) -> io::Result<T>>(
             &self,
             inode: Inode,
             cb: F,
@@ -638,7 +661,7 @@ mod private {
             self.access_value_mut(inode, |value| {
                 let handle = value
                     .take_handle()
-                    .ok_or_else(|| Error::NoHandlesAvailable(inode))?;
+                    .map_err(|_| Error::NoHandlesAvailable(inode))?;
                 let block = value.block_mut(handle)?;
                 let result = cb(handle, block);
                 if result.is_err() {
@@ -704,14 +727,14 @@ mod private {
                     Some(inode_lock) => inode_lock,
                     None => {
                         warn!("an attempt was made to forget non-existent inode {inode}");
-                        return Ok(())
+                        return Ok(());
                     }
                 };
                 let mut value = inode_lock.write().err_to_string()?;
                 value.decr_lookup_count(count)
             };
             if 1 == prev {
-                inodes.remove(&inode); 
+                inodes.remove(&inode);
             }
             Ok(())
         }
@@ -826,7 +849,7 @@ mod private {
             if flags & libc::O_DIRECTORY != 0 {
                 return Self::unsupported_flag("O_DIRECTORY");
             }
-            let handle = self.take_block_if_ok(inode, |handle, block| {
+            let handle = self.take_handle_if_ok(inode, |handle, block| {
                 let ctx = AuthzContext::new(ctx, block.meta());
                 if flags == libc::O_RDONLY || (flags & libc::O_RDWR) != 0 {
                     self.authorizer.can_read(&ctx)?;
@@ -865,16 +888,13 @@ mod private {
         ) -> io::Result<(Option<Self::Handle>, OpenOptions)> {
             debug!("Blocktree::opendir called on inode {inode}");
             let handle = self.access_value_mut(inode, |value| {
-                let dir = value.try_borrow_block(|block| {
+                value.try_borrow_block(|block| {
                     let ctx = AuthzContext::new(ctx, block.meta());
                     self.authorizer.can_exec(&ctx)?;
-                    block.seek(SeekFrom::Start(0))?;
-                    let dir: Directory = read_from(block)?;
-                    Ok(dir)
+                    Ok(())
                 })?;
-                let handle = value.next_handle;
-                value.next_handle += 1;
-                value.handle_values.insert(handle, HandleValue::new_dir(dir));
+                let handle = value.take_handle()?;
+                value.convert_to_dir(handle)?;
                 Ok(handle)
             })?;
             Ok((Some(handle), OpenOptions::empty()))
@@ -889,7 +909,7 @@ mod private {
         ) -> io::Result<()> {
             debug!("Blocktree::releasedir called for inode {inode}");
             self.access_value_mut(inode, |value| {
-                value.handle_values.remove(&handle);
+                value.give_handle(handle);
                 Ok(())
             })
         }
@@ -972,7 +992,10 @@ mod private {
         ) -> io::Result<usize> {
             debug!("Blocktree::write called called on inode {inode}");
             if flags as libc::c_int == libc::O_RDONLY {
-                return Err(io::Error::new(io::ErrorKind::PermissionDenied, "file is readonly"));
+                return Err(io::Error::new(
+                    io::ErrorKind::PermissionDenied,
+                    "file is readonly",
+                ));
             }
             let mut size: usize = size.try_into().box_err()?;
             self.access_block_mut(inode, handle, |block| {
@@ -1102,8 +1125,8 @@ mod private {
                         type_: entry.kind() as u32,
                         name: name.as_bytes(),
                     };
-                    size -= add_entry(dir_entry)?;
-                    if size <= 0 {
+                    size = size.saturating_sub(add_entry(dir_entry)?);
+                    if size == 0 {
                         break;
                     }
                 }
@@ -1428,10 +1451,7 @@ mod tests {
         abi::fuse_abi::CreateIn,
         api::filesystem::{Context, FileSystem, FsOptions},
     };
-    use std::{
-        io,
-        ffi::CString
-    };
+    use std::{ffi::CString, io};
     use tempdir::TempDir;
 
     use test_helpers::*;
@@ -1828,9 +1848,7 @@ mod tests {
             .expect("release failed");
 
         let flags = libc::O_RDONLY as u32;
-        let (handle, ..) = bt
-            .open(&ctx, inode, flags, 0)
-            .expect("open failed");
+        let (handle, ..) = bt.open(&ctx, inode, flags, 0).expect("open failed");
         let handle = handle.unwrap();
 
         const LEN: usize = 32;