Parcourir la source

Fixed a bug where Blocktree::read wasn't using the offset parameter.

Matthew Carr il y a 2 ans
Parent
commit
680fec54d3

+ 5 - 0
Cargo.toml

@@ -11,3 +11,8 @@ members = [
 
 [profile.bench]
 debug = true
+
+# Uncomment this to build the tests with optimizations. This provides better performance when using
+# the FUSE file system.
+#[profile.test]
+#opt-level = 3

+ 21 - 3
crates/btlib/src/accessor.rs

@@ -1,9 +1,9 @@
 use positioned_io::{ReadAt, Size, WriteAt};
-use std::io::{Read, Seek, Write};
+use std::io::{Read, Seek, SeekFrom, Write};
 
 use crate::{
-    sectored_buf::SectoredBuf, BlockMeta, Cursor, Decompose, FlushMeta, MetaAccess, Result,
-    SecretStream, Sectored, Split, TryCompose,
+    sectored_buf::SectoredBuf, BlockMeta, Cursor, Decompose, FlushMeta, MetaAccess, Positioned,
+    ReadDual, Result, SecretStream, Sectored, Split, TryCompose, TrySeek,
 };
 
 pub use private::Accessor;
@@ -94,6 +94,24 @@ mod private {
         }
     }
 
+    impl<T: ReadAt + AsRef<BlockMeta> + Size> ReadDual for Accessor<T> {
+        fn read_into<W: Write>(&mut self, write: W, count: usize) -> std::io::Result<usize> {
+            self.inner.read_into(write, count)
+        }
+    }
+
+    impl<T: Size> Positioned for Accessor<T> {
+        fn pos(&self) -> usize {
+            self.inner.pos()
+        }
+    }
+
+    impl<T: ReadAt + AsRef<BlockMeta> + Size> TrySeek for Accessor<T> {
+        fn try_seek(&mut self, seek_from: SeekFrom) -> std::io::Result<()> {
+            self.inner.try_seek(seek_from)
+        }
+    }
+
     impl<T: Size> Split<Accessor<&'static [u8]>, T> for Accessor<T> {
         fn split(self) -> (Accessor<&'static [u8]>, T) {
             let (sectored_buf, inner) = self.inner.split();

+ 138 - 75
crates/btlib/src/blocktree.rs

@@ -1,41 +1,42 @@
+use btserde::{read_from, write_to};
+use fuse_backend_rs::{
+    abi::fuse_abi::{stat64, statvfs64, CreateIn},
+    api::filesystem::{
+        Context, DirEntry as FuseDirEntry, Entry, FileSystem, FsOptions, OpenOptions, SetattrValid,
+    },
+};
+use log::{debug, error, warn};
+use positioned_io::Size;
+use serde::{Deserialize, Serialize};
+use std::{
+    cell::RefCell,
+    collections::hash_map::{self, HashMap},
+    ffi::CStr,
+    fmt::{Display, Formatter},
+    fs::File,
+    io::{self, Seek, SeekFrom, Write},
+    path::{Path, PathBuf},
+    sync::{
+        atomic::{AtomicU64, Ordering},
+        RwLock, RwLockWriteGuard,
+    },
+    time::Duration,
+};
+
+use crate::{
+    accessor::Accessor,
+    bterr,
+    crypto::{Creds, Decrypter, Signer},
+    error::{BtErr, DisplayErr, IoErr},
+    BlockAccessor, BlockError, BlockMeta, BlockOpenOptions, BlockPath, BlockReader, BlockRecord,
+    BoxInIoErr, DirEntry, DirEntryKind, Directory, Epoch, FileBlock, FlushMeta, MetaAccess,
+    MetaReader, Positioned, Principaled, ReadDual, Result, SeekFromExt, Split, TrySeek,
+};
+
 pub use private::{Blocktree, ModeAuthorizer};
 
 mod private {
-    use btserde::{read_from, write_to};
-    use fuse_backend_rs::{
-        abi::fuse_abi::{stat64, statvfs64, CreateIn},
-        api::filesystem::{
-            Context, DirEntry as FuseDirEntry, Entry, FileSystem, FsOptions, OpenOptions,
-            SetattrValid,
-        },
-    };
-    use log::{debug, error, warn};
-    use positioned_io::Size;
-    use serde::{Deserialize, Serialize};
-    use std::{
-        cell::RefCell,
-        collections::hash_map::{self, HashMap},
-        ffi::CStr,
-        fmt::{Display, Formatter},
-        fs::File,
-        io::{self, Read, Seek, SeekFrom, Write},
-        path::{Path, PathBuf},
-        sync::{
-            atomic::{AtomicU64, Ordering},
-            RwLock, RwLockWriteGuard,
-        },
-        time::Duration,
-    };
-
-    use crate::{
-        accessor::Accessor,
-        bterr,
-        crypto::{Creds, Decrypter, Signer},
-        error::{BtErr, DisplayErr, IoErr},
-        BlockAccessor, BlockError, BlockMeta, BlockOpenOptions, BlockPath, BlockReader,
-        BlockRecord, BoxInIoErr, DirEntry, DirEntryKind, Directory, Epoch, FileBlock, FlushMeta,
-        MetaAccess, MetaReader, Principaled, Result, SeekFromExt, Split,
-    };
+    use super::*;
 
     type Inode = u64;
     type Handle = u64;
@@ -1105,9 +1106,9 @@ mod private {
             _ctx: &Context,
             inode: Self::Inode,
             handle: Self::Handle,
-            w: &mut dyn fuse_backend_rs::api::filesystem::ZeroCopyWriter,
+            mut w: &mut dyn fuse_backend_rs::api::filesystem::ZeroCopyWriter,
             size: u32,
-            mut offset: u64,
+            offset: u64,
             _lock_owner: Option<u64>,
             flags: u32,
         ) -> io::Result<usize> {
@@ -1118,41 +1119,39 @@ mod private {
                     "file is write only",
                 ));
             }
-            let mut size: usize = size.try_into().box_err()?;
-            self.access_block(inode, handle, |block| {
-                let mut buf = [0u8; crate::SECTOR_SZ_DEFAULT];
-                let mut read = 0;
-                while size > 0 {
-                    let just_read = match block.read(&mut buf) {
-                        Ok(just_read) => just_read,
-                        Err(err) => {
-                            if read > 0 {
-                                error!("error while reading from block: {err}");
-                                return Ok(read);
+            let size: usize = size.try_into().box_err()?;
+            let read = self
+                .access_block(inode, handle, |block| {
+                    let pos = block.pos() as u64;
+                    if offset != pos {
+                        if let Err(err) = block.try_seek(SeekFrom::Start(offset)) {
+                            //  An error with `ErrorKind::Unsupported` means that the `SectoredBuf`
+                            // has unflushed data and it needs exclusive access to the block to
+                            // perform this seek because this data needs to be written.
+                            if let io::ErrorKind::Unsupported = err.kind() {
+                                return Ok(None);
                             } else {
                                 return Err(err.into());
                             }
                         }
-                    };
-                    if 0 == just_read {
-                        break;
                     }
-                    read += just_read;
-                    offset += just_read as u64;
-                    let filled = &buf[..just_read];
-                    if let Err(err) = w.write_all(filled) {
-                        if read > 0 {
-                            error!("error while writing: {err}");
-                            return Ok(read);
-                        } else {
-                            return Err(err.into());
-                        }
-                    }
-                    size -= filled.len();
+                    let read = block.read_into(&mut w, size).bterr()?;
+                    Ok(Some(read))
+                })
+                .io_err()?;
+            let read = match read {
+                Some(read) => read,
+                None => {
+                    // The offset of this read requires us to flush data buffered from a previous
+                    // write before seeking to a different sector, so we have to access the block
+                    // mutably.
+                    self.access_block_mut(inode, handle, |block| {
+                        block.seek(SeekFrom::Start(offset))?;
+                        block.read_into(w, size).bterr()
+                    })?
                 }
-                Ok(read)
-            })
-            .io_err()
+            };
+            Ok(read)
         }
 
         fn readdir(
@@ -1693,18 +1692,21 @@ mod private {
 
 #[cfg(test)]
 mod tests {
+    use super::*;
+
     use fuse_backend_rs::{
         abi::fuse_abi::CreateIn,
         api::filesystem::{Context, FileSystem, FsOptions},
     };
-    use std::{ffi::CString, io};
+    use std::ffi::CString;
     use tempdir::TempDir;
 
-    use test_helpers::*;
-
-    use crate::{crypto::ConcreteCreds, test_helpers, BlockMeta, Decompose};
-
-    use super::{private::SpecInodes, *};
+    use super::private::SpecInodes;
+    use crate::{
+        crypto::ConcreteCreds,
+        test_helpers::{node_creds, BtCursor},
+        BlockMeta, Decompose,
+    };
 
     /// Tests for the [ModeAuthorizer] struct.
     mod mode_authorizer_tests {
@@ -1725,8 +1727,8 @@ mod tests {
             const CTX_PID: libc::pid_t = 100;
 
             fn new(ctx_uid: u32, ctx_gid: u32, mode: u32) -> TestCase {
-                let mut meta = BlockMeta::new(&*test_helpers::NODE_CREDS)
-                    .expect("failed to create block metadata");
+                let mut meta =
+                    BlockMeta::new(node_creds()).expect("failed to create block metadata");
                 meta.body
                     .access_secrets(|secrets| {
                         secrets.uid = Self::BLOCK_UID;
@@ -1900,7 +1902,7 @@ mod tests {
         }
 
         fn creds() -> ConcreteCreds {
-            test_helpers::NODE_CREDS.clone()
+            node_creds().clone()
         }
 
         fn context(&self) -> Context {
@@ -2195,4 +2197,65 @@ mod tests {
         let result = bt.lookup(&ctx, root, &trash);
         assert_eq!(libc::ENOENT, result.err().unwrap().raw_os_error().unwrap());
     }
+
+    /// Tests that the `size` parameter actually limits the number of read bytes.
+    #[test]
+    fn read_with_smaller_size() {
+        const DATA: [u8; 8] = [0, 1, 2, 3, 4, 5, 6, 7];
+        let mut data = BtCursor::new(DATA);
+        let case = BtTestCase::new_empty();
+        let ctx = case.context();
+        let file = CString::new("file.txt").unwrap();
+        let bt = &case.bt;
+        let root = SpecInodes::RootDir.into();
+        let flags = libc::O_RDWR as u32;
+
+        let create_in = CreateIn {
+            flags,
+            mode: 0o644,
+            umask: 0,
+            fuse_flags: 0,
+        };
+        let (entry, handle, ..) = bt.create(&ctx, root, &file, create_in).unwrap();
+        let handle = handle.unwrap();
+        let inode = entry.inode;
+        let written = bt
+            .write(
+                &ctx,
+                inode,
+                handle,
+                &mut data,
+                DATA.len() as u32,
+                0,
+                None,
+                false,
+                flags,
+                0,
+            )
+            .unwrap();
+        assert_eq!(DATA.len(), written);
+        let new_pos = bt
+            .lseek(&ctx, inode, handle, 0, libc::SEEK_SET as u32)
+            .unwrap();
+        assert_eq!(0, new_pos);
+        data.rewind().unwrap();
+        const SIZE: usize = DATA.len() / 2;
+        let mut actual = BtCursor::new([0u8; DATA.len()]);
+        let read = bt
+            .read(
+                &ctx,
+                inode,
+                handle,
+                &mut actual,
+                SIZE as u32,
+                0,
+                None,
+                flags,
+            )
+            .unwrap();
+        assert_eq!(SIZE, read);
+
+        let actual = actual.into_inner();
+        assert_eq!([0, 1, 2, 3, 0, 0, 0, 0], actual);
+    }
 }

+ 2 - 2
crates/btlib/src/crypto/secret_stream.rs

@@ -1,5 +1,5 @@
-use std::io::{self, Read, Seek, SeekFrom, Write};
 use positioned_io::Size;
+use std::io::{self, Read, Seek, SeekFrom, Write};
 
 use crate::{
     bterr,
@@ -223,7 +223,7 @@ mod private {
 #[cfg(test)]
 mod tests {
     use crate::{
-        crypto::{SymKeyKind},
+        crypto::SymKeyKind,
         test_helpers::{Randomizer, SectoredCursor},
         SECTOR_SZ_DEFAULT,
     };

+ 22 - 0
crates/btlib/src/error.rs

@@ -36,6 +36,28 @@ macro_rules! btensure {
     };
 }
 
+/// Attempts to unwrap the given result.
+/// If the result is an error and the given count is positive, then the error is logged and
+/// `Ok($count)` is returned.
+/// If the result is an error and count is zero, then the `Err` variant is returned
+/// containing it.
+#[macro_export(crate)]
+macro_rules! suppress_err_if_non_zero {
+    ($count:expr, $result:expr) => {
+        match $result {
+            Ok(output) => output,
+            Err(err) => {
+                if $count > 0 {
+                    error!("{err}");
+                    return Ok($count);
+                } else {
+                    return Err(err.into());
+                }
+            }
+        }
+    };
+}
+
 pub type Result<T> = std::result::Result<T, Error>;
 
 /// The top-level error type used by this crate. This is just a newtype wrapper around

+ 44 - 12
crates/btlib/src/lib.rs

@@ -1,16 +1,16 @@
+pub mod accessor;
+mod block_path;
 pub mod blocktree;
+pub mod buf_reader;
 /// Code which enables cryptographic operations.
 pub mod crypto;
 pub mod error;
 pub mod log;
 pub mod msg;
-pub mod accessor;
-pub mod buf_reader;
 pub mod sectored_buf;
-mod block_path;
-mod trailered;
 #[cfg(test)]
 mod test_helpers;
+mod trailered;
 
 #[macro_use]
 extern crate static_assertions;
@@ -19,9 +19,9 @@ extern crate static_assertions;
 #[cfg(test)]
 extern crate lazy_static;
 
-use btserde::{read_from, write_to};
 use ::log::error;
 use brotli::{CompressorWriter, Decompressor};
+use btserde::{read_from, write_to};
 use fuse_backend_rs::abi::fuse_abi::{stat64, Attr};
 use positioned_io::{ReadAt, Size, WriteAt};
 use serde::{Deserialize, Serialize};
@@ -39,15 +39,15 @@ use std::{
 };
 use strum_macros::{Display, EnumDiscriminants, FromRepr};
 
-pub use error::{Error, Result};
+use accessor::Accessor;
 pub use block_path::{BlockPath, BlockPathError};
 use crypto::{
     AsymKeyPub, Ciphertext, Creds, Decrypter, DecrypterExt, Encrypter, EncrypterExt, HashKind,
     MerkleStream, PublicCreds, SecretStream, Sign, Signature, Signer, SymKey, SymKeyKind, VarHash,
 };
 use error::{BoxInIoErr, BtErr};
+pub use error::{Error, Result};
 use trailered::Trailered;
-use accessor::Accessor;
 
 #[derive(Debug)]
 pub enum BlockError {
@@ -130,8 +130,8 @@ impl<T: ReadAt + WriteAt + MetaAccess + Sectored + FlushMeta + ?Sized> Block for
 pub trait BlockReader: Read + Seek + AsRef<BlockMeta> + Size + Sectored {
     fn read_dir(&mut self) -> Result<Directory> {
         self.rewind()?;
-        let mut selphie = self;
-        let dir: Directory = read_from(&mut selphie)?;
+        let mut sich = self;
+        let dir: Directory = read_from(&mut sich)?;
         Ok(dir)
     }
 }
@@ -141,9 +141,9 @@ impl<T: Read + Seek + AsRef<BlockMeta> + Size + Sectored + ?Sized> BlockReader f
 pub trait BlockAccessor: BlockReader + Write + MetaAccess {
     fn write_dir(&mut self, dir: &Directory) -> Result<()> {
         self.rewind()?;
-        let mut selphie = self;
-        write_to(dir, &mut selphie)?;
-        selphie.flush()?;
+        let mut sich = self;
+        write_to(dir, &mut sich)?;
+        sich.flush()?;
         Ok(())
     }
 }
@@ -215,6 +215,38 @@ impl<T: Sectored + Size> Sectored for Cursor<T> {
     }
 }
 
+/// The `Read` trait requires the caller to supply the buffer to be read into. This trait is its
+/// dual in the sense that the trait implementor is expected to supply its own buffer, which a
+/// `Write` instance is given. This can be used to avoid copying in cases where the trait
+/// implementor is already buffering data, so it can give a reference to this buffer to the caller
+/// (for example in `SectoredBuf`)
+trait ReadDual: Read {
+    fn read_into<W: Write>(&mut self, write: W, count: usize) -> io::Result<usize>;
+}
+
+/// Trait for streams which can efficiently and infallibly return their current position.
+trait Positioned {
+    /// Returns the position of this stream (byte offset relative to the beginning).
+    fn pos(&self) -> usize;
+}
+
+impl<T: Positioned> Positioned for &T {
+    fn pos(&self) -> usize {
+        (**self).pos()
+    }
+}
+
+impl<T: Positioned> Positioned for &mut T {
+    fn pos(&self) -> usize {
+        (**self).pos()
+    }
+}
+
+trait TrySeek {
+    /// Attempts to seek to the given offset from the start of the stream.
+    fn try_seek(&mut self, seek_from: SeekFrom) -> io::Result<()>;
+}
+
 pub trait SizeExt: Size {
     fn size_or_err(&self) -> Result<u64> {
         self.size()?.ok_or_else(|| bterr!(BlockError::UnknownSize))

+ 157 - 48
crates/btlib/src/sectored_buf.rs

@@ -1,14 +1,19 @@
+use log::error;
+use positioned_io::Size;
+use std::io::{self, Read, Seek, SeekFrom, Write};
+
+use crate::{
+    bterr, suppress_err_if_non_zero, BlockError, BlockMeta, BoxInIoErr, Decompose, MetaAccess,
+    Positioned, ReadDual, ReadExt, Result, Sectored, SizeExt, Split, TryCompose, TrySeek,
+    EMPTY_SLICE,
+};
+
 pub use private::SectoredBuf;
 
 mod private {
-    use log::{error, warn};
-    use positioned_io::Size;
-    use std::io::{self, Read, Seek, SeekFrom, Write};
+    use crate::SeekFromExt;
 
-    use crate::{
-        bterr, BlockError, BlockMeta, BoxInIoErr, Decompose, MetaAccess, ReadExt, Result, Sectored,
-        Split, TryCompose, EMPTY_SLICE,
-    };
+    use super::*;
 
     /// A stream which buffers writes and read such that the inner stream only sees reads and writes
     /// of sector length buffers.
@@ -36,6 +41,25 @@ mod private {
         }
     }
 
+    /// Returns the slice of the internal buffer which is ready to be read from.
+    /// If the buffer all the bytes in the buffer have been consumed, then the buffer is refilled.
+    /// The returned slice will be empty if and only if there are no additional bytes in the
+    /// inner stream.
+    macro_rules! readable_slice {
+        ($self:expr) => {{
+            let pos = $self.buf_pos();
+            let end = $self.buf_end();
+            if pos == end {
+                match $self.fill_internal_buf() {
+                    Ok(byte_ct) => Ok(&$self.buf[..byte_ct]),
+                    Err(err) => Err(err),
+                }
+            } else {
+                Ok(&$self.buf[pos..end])
+            }
+        }};
+    }
+
     impl<T> SectoredBuf<T> {
         /// Returns a reference to the inner stream.
         pub fn get_ref(&self) -> &T {
@@ -238,27 +262,17 @@ mod private {
             }
 
             let dest_len_start = dest.len();
-            let mut src = {
-                let start = self.buf_pos();
-                let end = self.buf_end();
-                &self.buf[start..end]
-            };
+            let mut src = readable_slice!(self)?;
             while !dest.is_empty() {
                 if src.is_empty() {
-                    if self.pos >= self.len() {
-                        break;
-                    }
-                    let byte_ct = match self.fill_internal_buf() {
-                        Ok(byte_ct) => byte_ct,
-                        Err(err) => {
-                            warn!("SectoredBuf::full_internal_buf returned an error: {}", err);
-                            break;
-                        }
-                    };
-                    if 0 == byte_ct {
+                    src = suppress_err_if_non_zero!(
+                        dest_len_start - dest.len(),
+                        readable_slice!(self)
+                    );
+                    // If `src` is still empty, then we've reached the end of the stream.
+                    if src.is_empty() {
                         break;
                     }
-                    src = &self.buf[..byte_ct];
                 }
                 let sz = src.len().min(dest.len());
                 dest[..sz].copy_from_slice(&src[..sz]);
@@ -270,30 +284,23 @@ mod private {
         }
     }
 
-    impl<T: Seek + Read + Write + MetaAccess> Seek for SectoredBuf<T> {
-        fn seek(&mut self, seek_from: SeekFrom) -> io::Result<u64> {
+    impl<T: Seek + Read + AsRef<BlockMeta>> SectoredBuf<T> {
+        /// Seeks this stream to the given position.
+        /// If a seek to a different sector is needed then `pre_seek` is called before this seek
+        /// is performed. This can be used to flush buffered data, or to prevent the seek if a
+        /// flush can't be performed.
+        fn seek_impl<F: FnOnce(&mut Self) -> io::Result<()>>(
+            &mut self,
+            seek_from: SeekFrom,
+            pre_seek: F,
+        ) -> io::Result<u64> {
             let inner_pos = self.inner.stream_position()?;
-            let inner_pos_new = match seek_from {
-                SeekFrom::Start(rel_start) => rel_start,
-                SeekFrom::Current(rel_curr) => {
-                    if rel_curr > 0 {
-                        inner_pos + rel_curr as u64
-                    } else {
-                        inner_pos - rel_curr as u64
-                    }
-                }
-                SeekFrom::End(_) => {
-                    return Err(io::Error::new(
-                        io::ErrorKind::Unsupported,
-                        "seeking relative to the end of the stream is not supported",
-                    ))
-                }
-            };
+            let inner_pos_new = seek_from.abs(|| Ok(inner_pos), || self.size_or_err())?;
             let sect_index = self.sector_index(inner_pos);
             let sect_index_new = self.sector_index(inner_pos_new);
             let pos: u64 = self.pos.try_into().box_err()?;
             if sect_index != sect_index_new || pos == inner_pos {
-                self.flush()?;
+                pre_seek(self)?;
                 let sect_sz: u64 = self.sector_sz().try_into().box_err()?;
                 let seek_to = sect_index_new * sect_sz;
                 self.inner.seek(SeekFrom::Start(seek_to))?;
@@ -304,6 +311,12 @@ mod private {
         }
     }
 
+    impl<T: Seek + Read + Write + MetaAccess> Seek for SectoredBuf<T> {
+        fn seek(&mut self, seek_from: SeekFrom) -> io::Result<u64> {
+            self.seek_impl(seek_from, |sich| sich.flush())
+        }
+    }
+
     impl<U, T: AsRef<U>> AsRef<U> for SectoredBuf<T> {
         fn as_ref(&self) -> &U {
             self.inner.as_ref()
@@ -323,19 +336,62 @@ mod private {
             Ok(Some(self.inner.as_ref().body.secrets()?.size))
         }
     }
+
+    impl<T: Read + Seek + AsRef<BlockMeta>> ReadDual for SectoredBuf<T> {
+        fn read_into<W: Write>(&mut self, mut write: W, mut count: usize) -> io::Result<usize> {
+            let pos_start = self.pos;
+            let mut src = readable_slice!(self)?;
+            src = &src[..src.len().min(count)];
+            while count > 0 {
+                if src.is_empty() {
+                    src = suppress_err_if_non_zero!(self.pos - pos_start, readable_slice!(self));
+                    src = &src[..src.len().min(count)];
+                    // If `src` is still empty, then we've reached the end of the stream.
+                    if src.is_empty() {
+                        break;
+                    }
+                }
+                let written = write.write(src)?;
+                src = &src[written..];
+                self.pos += written;
+                count -= written;
+            }
+            Ok(self.pos - pos_start)
+        }
+    }
+
+    impl<T> Positioned for SectoredBuf<T> {
+        fn pos(&self) -> usize {
+            self.pos
+        }
+    }
+
+    impl<T: Read + Seek + AsRef<BlockMeta>> TrySeek for SectoredBuf<T> {
+        fn try_seek(&mut self, seek_from: SeekFrom) -> io::Result<()> {
+            self.seek_impl(seek_from, |sich| {
+                if sich.dirty {
+                    Err(io::Error::new(
+                        io::ErrorKind::Unsupported,
+                        "SectoredBuf::try_seek failed because it has unwritten data",
+                    ))
+                } else {
+                    Ok(())
+                }
+            })?;
+            Ok(())
+        }
+    }
 }
 
 #[cfg(test)]
 mod tests {
-    use std::io::{Read, Seek, SeekFrom, Write};
+    use super::*;
 
     use crate::{
-        test_helpers::{read_check, write_fill, Randomizer, SectoredCursor},
-        Decompose, ReadExt, Sectored, TryCompose, SECTOR_SZ_DEFAULT,
+        test_helpers::{read_check, write_fill, BtCursor, Randomizer, SectoredCursor},
+        SECTOR_SZ_DEFAULT,
     };
 
-    use super::*;
-
     fn make_sectored_buf(sect_sz: usize, sect_ct: usize) -> SectoredBuf<SectoredCursor<Vec<u8>>> {
         SectoredBuf::new()
             .try_compose(SectoredCursor::new(vec![0u8; sect_sz * sect_ct], sect_sz))
@@ -529,4 +585,57 @@ mod tests {
         sectored.read(&mut actual).expect("read failed");
         assert_eq!(expected, actual);
     }
+
+    #[test]
+    fn read_into_count_limits_read_bytes() {
+        const DATA: [u8; 8] = [0, 1, 2, 3, 4, 5, 6, 7];
+        let mut sectored = SectoredBuf::new()
+            .try_compose(SectoredCursor::new(Vec::new(), DATA.len()))
+            .unwrap();
+
+        sectored.write(&DATA).unwrap();
+        sectored.rewind().unwrap();
+        let mut actual = BtCursor::new([0u8; DATA.len()]);
+        let read = sectored.read_into(&mut actual, DATA.len() - 1).unwrap();
+        assert_eq!(DATA.len() - 1, read);
+
+        assert_eq!([0, 1, 2, 3, 4, 5, 6, 0], actual.into_inner());
+    }
+
+    #[test]
+    fn read_into_read_spans_multiple_sectors() {
+        const DATA: [u8; 8] = [0, 1, 2, 3, 4, 5, 6, 7];
+        let mut sectored = SectoredBuf::new()
+            .try_compose(SectoredCursor::new(Vec::new(), DATA.len()))
+            .unwrap();
+
+        sectored.write(&DATA).unwrap();
+        sectored.write(&DATA).unwrap();
+        sectored.rewind().unwrap();
+        const ACTUAL_LEN: usize = DATA.len() + DATA.len() / 2;
+        let mut actual = BtCursor::new([0u8; ACTUAL_LEN]);
+        let read = sectored.read_into(&mut actual, ACTUAL_LEN).unwrap();
+        assert_eq!(ACTUAL_LEN, read);
+
+        assert_eq!([0, 1, 2, 3, 4, 5, 6, 7, 0, 1, 2, 3], actual.into_inner());
+    }
+
+    /// Tests that a read asking for more bytes than the number available returns the number
+    /// available.
+    #[test]
+    fn read_into_read_past_len() {
+        const DATA: [u8; 8] = [0, 1, 2, 3, 4, 5, 6, 7];
+        let mut sectored = SectoredBuf::new()
+            .try_compose(SectoredCursor::new(Vec::new(), DATA.len()))
+            .unwrap();
+
+        sectored.write(&DATA).unwrap();
+        sectored.rewind().unwrap();
+        const ACTUAL_LEN: usize = DATA.len() + 1;
+        let mut actual = BtCursor::new([0u8; ACTUAL_LEN]);
+        let read = sectored.read_into(&mut actual, ACTUAL_LEN).unwrap();
+        assert_eq!(DATA.len(), read);
+
+        assert_eq!([0, 1, 2, 3, 4, 5, 6, 7, 0], actual.into_inner());
+    }
 }