refactor

* Thanks clippy
* typed errors

Author: Byron

gix-features/src/lib.rs

@@ -12,7 +12,7 @@
     doc = ::document_features::document_features!()
 )]
 #![cfg_attr(all(doc, feature = "document-features"), feature(doc_cfg, doc_auto_cfg))]
-#![deny(rust_2018_idioms)]
+#![deny(rust_2018_idioms, missing_docs)]
 
 ///
 pub mod cache;

gix-features/src/zlib/mod.rs

@@ -1,20 +1,30 @@
-use std::fmt::Display;
+use std::ffi::c_int;
 
+/// A type to hold all state needed for decompressing a ZLIB encoded stream.
 pub struct Decompress(Box<libz_rs_sys::z_stream>);
 
 unsafe impl Sync for Decompress {}
 unsafe impl Send for Decompress {}
 
+impl Default for Decompress {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
 impl Decompress {
+    /// The amount of bytes consumed from the input so far.
     pub fn total_in(&self) -> u64 {
-        self.0.total_in
+        self.0.total_in as _
     }
 
+    /// The amount of decompressed bytes that have been written to the output thus far.
     pub fn total_out(&self) -> u64 {
-        self.0.total_out
+        self.0.total_out as _
     }
 
-    pub fn new(_zlib_header: bool) -> Self {
+    /// Create a new instance. Note that it allocates in various ways and thus should be re-used.
+    pub fn new() -> Self {
         let mut this = Box::new(libz_rs_sys::z_stream::default());
 
         unsafe {
@@ -28,10 +38,12 @@ impl Decompress {
         Self(this)
     }
 
-    pub fn reset(&mut self, _: bool) {
+    /// Reset the state to allow handling a new stream.
+    pub fn reset(&mut self) {
         unsafe { libz_rs_sys::inflateReset(&mut *self.0) };
     }
 
+    /// Decompress `input` and write all decompressed bytes into `output`, with `flush` defining some details about this.
     pub fn decompress(
         &mut self,
         input: &[u8],
@@ -49,11 +61,10 @@ impl Decompress {
             libz_rs_sys::Z_BUF_ERROR => Ok(Status::BufError),
             libz_rs_sys::Z_STREAM_END => Ok(Status::StreamEnd),
 
-            libz_rs_sys::Z_STREAM_ERROR => Err(DecompressError("stream error")),
-            libz_rs_sys::Z_DATA_ERROR => Err(DecompressError("data error")),
-            libz_rs_sys::Z_MEM_ERROR => Err(DecompressError("insufficient memory")),
-            libz_rs_sys::Z_NEED_DICT => Err(DecompressError("need dictionary")),
-            c => panic!("unknown return code: {}", c),
+            libz_rs_sys::Z_STREAM_ERROR => Err(DecompressError::StreamError),
+            libz_rs_sys::Z_DATA_ERROR => Err(DecompressError::DataError),
+            libz_rs_sys::Z_MEM_ERROR => Err(DecompressError::InsufficientMemory),
+            err => Err(DecompressError::Unknown { err }),
         }
     }
 }
@@ -64,19 +75,29 @@ impl Drop for Decompress {
     }
 }
 
+/// The error produced by [`Decompress::decompress()`].
 #[derive(Debug, thiserror::Error)]
-pub struct DecompressError(&'static str);
-
-impl Display for DecompressError {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        f.write_str(self.0)
-    }
+#[allow(missing_docs)]
+pub enum DecompressError {
+    #[error("stream error")]
+    StreamError,
+    #[error("Not enough memory")]
+    InsufficientMemory,
+    #[error("Invalid input data")]
+    DataError,
+    #[error("An unknown error occurred: {err}")]
+    Unknown { err: c_int },
 }
 
+/// The status returned by [`Decompress::decompress()`].
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum Status {
+    /// The decompress operation went well. Not to be confused with `StreamEnd`, so one can continue
+    /// the decompression.
     Ok,
+    /// An error occurred when decompression.
     BufError,
+    /// The stream was fully decompressed.
     StreamEnd,
 }
 
@@ -123,19 +144,12 @@ pub mod inflate {
 }
 
 /// Decompress a few bytes of a zlib stream without allocation
+#[derive(Default)]
 pub struct Inflate {
     /// The actual decompressor doing all the work.
     pub state: Decompress,
 }
 
-impl Default for Inflate {
-    fn default() -> Self {
-        Inflate {
-            state: Decompress::new(true),
-        }
-    }
-}
-
 impl Inflate {
     /// Run the decompressor exactly once. Cannot be run multiple times
     pub fn once(&mut self, input: &[u8], out: &mut [u8]) -> Result<(Status, usize, usize), inflate::Error> {
@@ -151,7 +165,7 @@ impl Inflate {
 
     /// Ready this instance for decoding another data stream.
     pub fn reset(&mut self) {
-        self.state.reset(true);
+        self.state.reset();
     }
 }
 

gix-features/src/zlib/stream/deflate/mod.rs

@@ -1,4 +1,5 @@
 use crate::zlib::Status;
+use std::ffi::c_int;
 
 const BUF_SIZE: usize = 4096 * 8;
 
@@ -24,20 +25,30 @@ where
     }
 }
 
+/// Hold all state needed for compressing data.
 pub struct Compress(Box<libz_rs_sys::z_stream>);
 
 unsafe impl Sync for Compress {}
 unsafe impl Send for Compress {}
 
+impl Default for Compress {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
 impl Compress {
+    /// The number of bytes that were read from the input.
     pub fn total_in(&self) -> u64 {
-        self.0.total_in
+        self.0.total_in as _
     }
 
+    /// The number of compressed bytes that were written to the output.
     pub fn total_out(&self) -> u64 {
-        self.0.total_out
+        self.0.total_out as _
     }
 
+    /// Create a new instance - this allocates so should be done with care.
     pub fn new() -> Self {
         let mut this = Box::new(libz_rs_sys::z_stream::default());
 
@@ -53,10 +64,12 @@ impl Compress {
         Self(this)
     }
 
+    /// Prepare the instance for a new stream.
     pub fn reset(&mut self) {
         unsafe { libz_rs_sys::deflateReset(&mut *self.0) };
     }
 
+    /// Compress `input` and write compressed bytes to `output`, with `flush` controlling additional characteristics.
     pub fn compress(&mut self, input: &[u8], output: &mut [u8], flush: FlushCompress) -> Result<Status, CompressError> {
         self.0.avail_in = input.len() as _;
         self.0.avail_out = output.len() as _;
@@ -69,8 +82,9 @@ impl Compress {
             libz_rs_sys::Z_BUF_ERROR => Ok(Status::BufError),
             libz_rs_sys::Z_STREAM_END => Ok(Status::StreamEnd),
 
-            libz_rs_sys::Z_STREAM_ERROR => Err(CompressError("stream error")),
-            _ => todo!(),
+            libz_rs_sys::Z_STREAM_ERROR => Err(CompressError::StreamError),
+            libz_rs_sys::Z_MEM_ERROR => Err(CompressError::InsufficientMemory),
+            err => Err(CompressError::Unknown { err }),
         }
     }
 }
@@ -81,13 +95,17 @@ impl Drop for Compress {
     }
 }
 
+/// The error produced by [`Compress::compress()`].
 #[derive(Debug, thiserror::Error)]
-pub struct CompressError(&'static str);
-
-impl std::fmt::Display for CompressError {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        f.write_str(self.0)
-    }
+#[error("{msg}")]
+#[allow(missing_docs)]
+pub enum CompressError {
+    #[error("stream error")]
+    StreamError,
+    #[error("Not enough memory")]
+    InsufficientMemory,
+    #[error("An unknown error occurred: {err}")]
+    Unknown { err: c_int },
 }
 
 /// Values which indicate the form of flushing to be used when compressing

gix-features/src/zlib/stream/deflate/tests.rs

@@ -22,7 +22,7 @@ mod deflate_stream {
     {
         pub fn from_read(read: R) -> InflateReader<R> {
             InflateReader {
-                decompressor: Decompress::new(true),
+                decompressor: Decompress::new(),
                 inner: read,
             }
         }

gix-pack/src/data/input/bytes_to_entries.rs

@@ -62,7 +62,7 @@ where
         );
         Ok(BytesToEntriesIter {
             read,
-            decompressor: Decompress::new(true),
+            decompressor: Decompress::new(),
             compressed,
             offset: 12,
             had_error: false,
@@ -101,7 +101,7 @@ where
 
         // Decompress object to learn its compressed bytes
         let compressed_buf = self.compressed_buf.take().unwrap_or_else(|| Vec::with_capacity(4096));
-        self.decompressor.reset(true);
+        self.decompressor.reset();
         let mut decompressed_reader = DecompressRead {
             inner: read_and_pass_to(
                 &mut self.read,

gix/tests/gix/repository/mod.rs

@@ -134,7 +134,7 @@ mod dirwalk {
 #[test]
 fn size_in_memory() {
     let actual_size = std::mem::size_of::<Repository>();
-    let limit = 1200;
+    let limit = 1250;
     assert!(
         actual_size <= limit,
         "size of Repository shouldn't change without us noticing, it's meant to be cloned: should have been below {limit:?}, was {actual_size} (bigger on windows)"

gix/tests/gix/revision/spec/from_bytes/mod.rs

@@ -126,8 +126,8 @@ fn bad_objects_are_valid_until_they_are_actually_read_from_the_odb() {
             Spec::from_id(hex_to_id("cafea31147e840161a1860c50af999917ae1536b").attach(&repo))
         );
         assert_eq!(
-            &format!("{:?}", parse_spec("cafea^{object}", &repo).unwrap_err())[..80],
-            r#"FindObject(Find(Loose(DecompressFile { source: Inflate(DecompressError("data err"#
+            &format!("{:?}", parse_spec("cafea^{object}", &repo).unwrap_err())[..65],
+            r#"FindObject(Find(Loose(DecompressFile { source: Inflate(DataError)"#
         );
     }
 }