From 21d0a854f6bc7e971da07fada2c476ab9e8eca1b Mon Sep 17 00:00:00 2001 From: George Steel Date: Wed, 7 Jan 2026 20:08:10 -0500 Subject: [PATCH] Allow ArchiveHandle to own its data --- rc-zip-cli/src/main.rs | 2 +- rc-zip-sync/benches/read.rs | 6 +- rc-zip-sync/examples/byte_count.rs | 2 +- rc-zip-sync/examples/self_extracting.rs | 2 +- rc-zip-sync/src/read_zip.rs | 184 ++++++++++++++++-------- rc-zip-sync/tests/integration_tests.rs | 2 +- 6 files changed, 135 insertions(+), 63 deletions(-) diff --git a/rc-zip-cli/src/main.rs b/rc-zip-cli/src/main.rs index 26d2409..528c2f1 100644 --- a/rc-zip-cli/src/main.rs +++ b/rc-zip-cli/src/main.rs @@ -136,7 +136,7 @@ fn info(out: &mut impl io::Write, archive: &Archive) -> io::Result<()> { fn list( out: &mut impl io::Write, - archive: &ArchiveHandle<'_, File>, + archive: &ArchiveHandle, verbose: bool, ) -> io::Result<()> { for entry in archive.entries() { diff --git a/rc-zip-sync/benches/read.rs b/rc-zip-sync/benches/read.rs index c58eba6..5f87247 100644 --- a/rc-zip-sync/benches/read.rs +++ b/rc-zip-sync/benches/read.rs @@ -1,4 +1,4 @@ -use std::{hint::black_box, time::Duration}; +use std::{hint::black_box, ops::Deref, time::Duration}; use divan::{bench, counter::ItemsCount, Bencher, Divan}; use rc_zip_corpus::test_cases; @@ -15,10 +15,10 @@ fn main() { fn archive_entries(bencher: Bencher, name: &'static str) { let case = test_cases().into_iter().find(|c| c.name == name).unwrap(); let zip_contents = case.bytes(); - let archive = zip_contents.read_zip().unwrap(); + let archive = zip_contents.deref().read_zip().unwrap(); let num_entries = ItemsCount::new(archive.entries().count()); bencher .counter(num_entries) - .bench(|| black_box(&zip_contents).read_zip().unwrap()); + .bench(|| black_box(zip_contents.deref()).read_zip().unwrap()); } diff --git a/rc-zip-sync/examples/byte_count.rs b/rc-zip-sync/examples/byte_count.rs index 363f7a9..8e8a29a 100644 --- a/rc-zip-sync/examples/byte_count.rs +++ b/rc-zip-sync/examples/byte_count.rs @@ -60,7 +60,7 @@ impl Counts { /// The work is split across a pool of worker threads where each worker takes turns fetching an /// entry from the archive to then read over the entry and reduce it down to counts. Then the final /// counts are totaled together as each worker finishes. -fn byte_count_multi_threaded(archive: &ArchiveHandle<'_, File>) -> Counts { +fn byte_count_multi_threaded(archive: &ArchiveHandle) -> Counts { let mut total_counts = Counts::new(); let entries = Mutex::new(archive.entries()); let num_workers = thread::available_parallelism().unwrap(); diff --git a/rc-zip-sync/examples/self_extracting.rs b/rc-zip-sync/examples/self_extracting.rs index c7ab76a..c4ac9c9 100644 --- a/rc-zip-sync/examples/self_extracting.rs +++ b/rc-zip-sync/examples/self_extracting.rs @@ -41,7 +41,7 @@ fn main() -> Result<(), Error> { Ok(()) } -fn extract(archive: &ArchiveHandle<'_, File>) -> Result<(), Error> { +fn extract(archive: &ArchiveHandle) -> Result<(), Error> { for entry in archive.entries() { println!("extracting {}", entry.name); let Some(entry_name) = entry.sanitized_name() else { diff --git a/rc-zip-sync/src/read_zip.rs b/rc-zip-sync/src/read_zip.rs index 1e72ce9..f10c28d 100644 --- a/rc-zip-sync/src/read_zip.rs +++ b/rc-zip-sync/src/read_zip.rs @@ -6,7 +6,7 @@ use tracing::trace; use crate::entry_reader::EntryReader; use crate::streaming_entry_reader::StreamingEntryReader; -use std::{io::Read, ops::Deref}; +use std::{io::Read, ops::Deref, sync::Arc}; /// A trait for reading something as a zip archive /// @@ -16,7 +16,7 @@ pub trait ReadZipWithSize { type File: HasCursor; /// Reads self as a zip archive. - fn read_zip_with_size(&self, size: u64) -> Result, Error>; + fn read_zip_with_size(self, size: u64) -> Result, Error>; } /// A trait for reading something as a zip archive when we can tell size from @@ -28,7 +28,7 @@ pub trait ReadZip { type File: HasCursor; /// Reads self as a zip archive. - fn read_zip(&self) -> Result, Error>; + fn read_zip(self) -> Result, Error>; } struct CursorState<'a, F: HasCursor + 'a> { @@ -52,64 +52,66 @@ impl<'a, F: HasCursor + 'a> CursorState<'a, F> { impl ReadZipWithSize for F where - F: HasCursor, + F: HasCursor + Sized, { type File = F; - fn read_zip_with_size(&self, size: u64) -> Result, Error> { - let mut cstate: Option> = None; - - let mut fsm = ArchiveFsm::new(size); - loop { - if let Some(offset) = fsm.wants_read() { - trace!(%offset, "read_zip_with_size: wants_read, space len = {}", fsm.space().len()); - - let mut cstate_next = match cstate.take() { - // all good, re-using - Some(cstate) if cstate.offset == offset => cstate, - Some(cstate) => { - trace!(%offset, %cstate.offset, "read_zip_with_size: making new cursor (had wrong offset)"); - CursorState::try_new(self, offset, size)? - } - None => { - trace!(%offset, "read_zip_with_size: making new cursor (had none)"); - CursorState::try_new(self, offset, size)? - } - }; - - match cstate_next.cursor.read(fsm.space()) { - Ok(read_bytes) => { - cstate_next.offset += read_bytes as u64; - cstate = Some(cstate_next); - - trace!(%read_bytes, "read_zip_with_size: read"); - if read_bytes == 0 { - return Err(Error::IO(std::io::ErrorKind::UnexpectedEof.into())); + fn read_zip_with_size(self, size: u64) -> Result, Error> { + let archive = { + let mut cstate: Option> = None; + let mut fsm = ArchiveFsm::new(size); + loop { + if let Some(offset) = fsm.wants_read() { + trace!(%offset, "read_zip_with_size: wants_read, space len = {}", fsm.space().len()); + + let mut cstate_next = match cstate.take() { + // all good, re-using + Some(cstate) if cstate.offset == offset => cstate, + Some(cstate) => { + trace!(%offset, %cstate.offset, "read_zip_with_size: making new cursor (had wrong offset)"); + CursorState::try_new(&self, offset, size)? + } + None => { + trace!(%offset, "read_zip_with_size: making new cursor (had none)"); + CursorState::try_new(&self, offset, size)? } - fsm.fill(read_bytes); + }; + + match cstate_next.cursor.read(fsm.space()) { + Ok(read_bytes) => { + cstate_next.offset += read_bytes as u64; + cstate = Some(cstate_next); + + trace!(%read_bytes, "read_zip_with_size: read"); + if read_bytes == 0 { + return Err(Error::IO(std::io::ErrorKind::UnexpectedEof.into())); + } + fsm.fill(read_bytes); + } + Err(err) => return Err(Error::IO(err)), } - Err(err) => return Err(Error::IO(err)), } - } - fsm = match fsm.process()? { - FsmResult::Done(archive) => { - trace!("read_zip_with_size: done"); - return Ok(ArchiveHandle { - file: self, - archive, - }); + fsm = match fsm.process()? { + FsmResult::Done(archive) => { + trace!("read_zip_with_size: done"); + break archive; + } + FsmResult::Continue(fsm) => fsm, } - FsmResult::Continue(fsm) => fsm, } - } + }; + return Ok(ArchiveHandle { + file: self, + archive, + }); } } impl ReadZip for &[u8] { type File = Self; - fn read_zip(&self) -> Result, Error> { + fn read_zip(self) -> Result, Error> { self.read_zip_with_size(self.len() as u64) } } @@ -117,8 +119,27 @@ impl ReadZip for &[u8] { impl ReadZip for Vec { type File = Self; - fn read_zip(&self) -> Result, Error> { - self.read_zip_with_size(self.len() as u64) + fn read_zip(self) -> Result, Error> { + let len = self.len(); + self.read_zip_with_size(len as u64) + } +} + +impl ReadZip for Box<[u8]> { + type File = Self; + + fn read_zip(self) -> Result, Error> { + let len = self.len(); + self.read_zip_with_size(len as u64) + } +} + +impl ReadZip for Arc<[u8]> { + type File = Self; + + fn read_zip(self) -> Result, Error> { + let len = self.len(); + self.read_zip_with_size(len as u64) } } @@ -127,15 +148,15 @@ impl ReadZip for Vec { /// This only contains metadata for the archive and its entries. Separate /// readers can be created for arbitraries entries on-demand using /// [EntryHandle::reader]. -pub struct ArchiveHandle<'a, F> +pub struct ArchiveHandle where F: HasCursor, { - file: &'a F, + file: F, archive: Archive, } -impl Deref for ArchiveHandle<'_, F> +impl Deref for ArchiveHandle where F: HasCursor, { @@ -146,14 +167,14 @@ where } } -impl ArchiveHandle<'_, F> +impl ArchiveHandle where F: HasCursor, { /// Iterate over all files in this zip, read from the central directory. pub fn entries(&self) -> impl Iterator> { self.archive.entries().map(move |entry| EntryHandle { - file: self.file, + file: &self.file, entry, }) } @@ -165,7 +186,7 @@ where .entries() .find(|&x| x.name == name.as_ref()) .map(|entry| EntryHandle { - file: self.file, + file: &self.file, entry, }) } @@ -214,7 +235,7 @@ pub trait HasCursor { fn cursor_at(&self, offset: u64) -> Self::Cursor<'_>; } -impl HasCursor for &[u8] { +impl HasCursor for [u8] { type Cursor<'a> = &'a [u8] where @@ -236,6 +257,37 @@ impl HasCursor for Vec { } } +impl HasCursor for &T { + type Cursor<'a> + = T::Cursor<'a> + where + Self: 'a; + fn cursor_at(&self, offset: u64) -> Self::Cursor<'_> { + let inner: &T = self; + inner.cursor_at(offset) + } +} + +impl HasCursor for Arc { + type Cursor<'a> + = T::Cursor<'a> + where + Self: 'a; + fn cursor_at(&self, offset: u64) -> Self::Cursor<'_> { + self.deref().cursor_at(offset) + } +} + +impl HasCursor for Box { + type Cursor<'a> + = T::Cursor<'a> + where + Self: 'a; + fn cursor_at(&self, offset: u64) -> Self::Cursor<'_> { + self.deref().cursor_at(offset) + } +} + #[cfg(feature = "file")] impl HasCursor for std::fs::File { type Cursor<'a> @@ -252,7 +304,27 @@ impl HasCursor for std::fs::File { impl ReadZip for std::fs::File { type File = Self; - fn read_zip(&self) -> Result, Error> { + fn read_zip(self) -> Result, Error> { + let size = self.metadata()?.len(); + self.read_zip_with_size(size) + } +} + +#[cfg(feature = "file")] +impl ReadZip for &std::fs::File { + type File = Self; + + fn read_zip(self) -> Result, Error> { + let size = self.metadata()?.len(); + self.read_zip_with_size(size) + } +} + +#[cfg(feature = "file")] +impl ReadZip for Arc { + type File = Self; + + fn read_zip(self) -> Result, Error> { let size = self.metadata()?.len(); self.read_zip_with_size(size) } diff --git a/rc-zip-sync/tests/integration_tests.rs b/rc-zip-sync/tests/integration_tests.rs index 25d3b43..90455a7 100644 --- a/rc-zip-sync/tests/integration_tests.rs +++ b/rc-zip-sync/tests/integration_tests.rs @@ -7,7 +7,7 @@ use std::{ io::{self, Read}, }; -fn check_case(test: &Case, archive: Result, Error>) { +fn check_case(test: &Case, archive: Result, Error>) { rc_zip_corpus::check_case(test, archive.as_ref().map(|ar| -> &Archive { ar })); let archive = match archive { Ok(archive) => archive,