fix(inflate): use function instead of lookup table for distance extra bits for tiny space/perf saving and fix clippy warnings

Author: oyvindln

miniz_oxide/src/inflate/core.rs

@@ -99,7 +99,7 @@ const MAX_HUFF_SYMBOLS_0: usize = 288;
 /// The length of the second (distance) huffman table.
 const MAX_HUFF_SYMBOLS_1: usize = 32;
 /// The length of the last (huffman code length) huffman table.
-const _MAX_HUFF_SYMBOLS_2: usize = 19;
+// const _MAX_HUFF_SYMBOLS_2: usize = 19;
 /// The maximum length of a code that can be looked up in the fast lookup table.
 const FAST_LOOKUP_BITS: u8 = 10;
 /// The size of the fast lookup table.
@@ -337,7 +337,6 @@ impl State {
 
 use self::State::*;
 
-// Not sure why miniz uses 32-bit values for these, maybe alignment/cache again?
 // # Optimization
 // We add a extra value at the end and make the tables 32 elements long
 // so we can use a mask to avoid bounds checks.
@@ -362,18 +361,20 @@ const LENGTH_EXTRA: [u8; 32] = [
 
 /// Base length for each distance code.
 #[rustfmt::skip]
-const DIST_BASE: [u16; 32] = [
+const DIST_BASE: [u16; 30] = [
     1,    2,    3,    4,    5,    7,      9,      13,     17,     25,    33,
     49,   65,   97,   129,  193,  257,    385,    513,    769,    1025,  1537,
-    2049, 3073, 4097, 6145, 8193, 12_289, 16_385, 24_577, 32_768, 32_768
+    2049, 3073, 4097, 6145, 8193, 12_289, 16_385, 24_577
 ];
 
-/// Number of extra bits for each distance code.
-#[rustfmt::skip]
-const DIST_EXTRA: [u8; 32] = [
-    0, 0, 0, 0, 1, 1, 2,  2,  3,  3,  4,  4,  5,  5,  6,  6,
-    7, 7, 8, 8, 9, 9, 10, 10, 11, 11, 12, 12, 13, 13, 13, 13
-];
+/// Get the number of extra bits used for a distance code.
+/// (Code numbers above `NUM_DISTANCE_CODES` will give some garbage
+/// value.)
+const fn num_extra_bits_for_distance_code(code: u8) -> u8 {
+    // This can be easily calculated without a lookup.
+    let c = code >> 1;
+    c - (c != 0) as u8
+}
 
 /// The mask used when indexing the base/extra arrays.
 const BASE_EXTRA_MASK: usize = 32 - 1;
@@ -1092,7 +1093,7 @@ fn decompress_fast(
                     break 'o TINFLStatus::Failed;
                 }
 
-                l.num_extra = u32::from(DIST_EXTRA[symbol as usize]);
+                l.num_extra = u32::from(num_extra_bits_for_distance_code(symbol as u8));
                 l.dist = u32::from(DIST_BASE[symbol as usize]);
             } else {
                 state.begin(InvalidCodeLen);
@@ -1149,18 +1150,18 @@ fn decompress_fast(
 ///
 /// * The offset given by `out_pos` indicates where in the output buffer slice writing should start.
 /// * If [`TINFL_FLAG_USING_NON_WRAPPING_OUTPUT_BUF`] is not set, the output buffer is used in a
-/// wrapping manner, and it's size is required to be a power of 2.
+///   wrapping manner, and it's size is required to be a power of 2.
 /// * The decompression function normally needs access to 32KiB of the previously decompressed data
-///(or to the beginning of the decompressed data if less than 32KiB has been decompressed.)
+///   (or to the beginning of the decompressed data if less than 32KiB has been decompressed.)
 ///     - If this data is not available, decompression may fail.
 ///     - Some deflate compressors allow specifying a window size which limits match distances to
-/// less than this, or alternatively an RLE mode where matches will only refer to the previous byte
-/// and thus allows a smaller output buffer. The window size can be specified in the zlib
-/// header structure, however, the header data should not be relied on to be correct.
+///       less than this, or alternatively an RLE mode where matches will only refer to the previous byte
+///       and thus allows a smaller output buffer. The window size can be specified in the zlib
+///       header structure, however, the header data should not be relied on to be correct.
 ///
 /// `flags` indicates settings and status to the decompression function.
 /// * The [`TINFL_FLAG_HAS_MORE_INPUT`] has to be specified if more compressed data is to be provided
-/// in a subsequent call to this function.
+///   in a subsequent call to this function.
 /// * See the the [`inflate_flags`] module for details on other flags.
 ///
 /// # Returns
@@ -1177,7 +1178,7 @@ pub fn decompress(
     flags: u32,
 ) -> (TINFLStatus, usize, usize) {
     let out_buf_size_mask = if flags & TINFL_FLAG_USING_NON_WRAPPING_OUTPUT_BUF != 0 {
-        usize::max_value()
+        usize::MAX
     } else {
         // In the case of zero len, any attempt to write would produce HasMoreOutput,
         // so to gracefully process the case of there really being no output,
@@ -1610,16 +1611,19 @@ pub fn decompress(
                 // Try to read a huffman code from the input buffer and look up what
                 // length code the decoded symbol refers to.
                 decode_huffman_code(r, &mut l, DIST_TABLE, flags, &mut in_iter, |_r, l, symbol| {
+                    // # Optimizaton - transform the value into usize here before the check so
+                    // the compiler can optimize the bounds check later - ideally it should
+                    // know that the value can't be negative from earlier in the
+                    // decode_huffman_code function but it seems it may not be able
+                    // to make the assumption that it can't be negative and thus
+                    // overflow if it's converted after the check.
+                    let symbol = symbol as usize;
                     if symbol > 29 {
                         // Invalid distance code.
                         return Action::Jump(InvalidDist)
                     }
-                    // # Optimization
-                    // Mask the value to avoid bounds checks
-                    // We could use get_unchecked later if can statically verify that
-                    // this will never go out of bounds.
-                    l.num_extra = u32::from(DIST_EXTRA[symbol as usize & BASE_EXTRA_MASK]);
-                    l.dist = u32::from(DIST_BASE[symbol as usize & BASE_EXTRA_MASK]);
+                    l.num_extra = u32::from(num_extra_bits_for_distance_code(symbol as u8));
+                    l.dist = u32::from(DIST_BASE[symbol]);
                     if l.num_extra != 0 {
                         // ReadEXTRA_BITS_DISTACNE
                         Action::Jump(ReadExtraBitsDistance)
@@ -2051,4 +2055,18 @@ mod test {
         let res = decompress(&mut r, &encoded, &mut output_buf, 0, flags);
         assert_eq!(res, (TINFLStatus::HasMoreOutput, 2, 0));
     }
+
+    #[test]
+    fn dist_extra_bits() {
+        use self::num_extra_bits_for_distance_code;
+        // Number of extra bits for each distance code.
+        const DIST_EXTRA: [u8; 29] = [
+            0, 0, 0, 0, 1, 1, 2, 2, 3, 3, 4, 4, 5, 5, 6, 6, 7, 7, 8, 8, 9, 9, 10, 10, 11, 11, 12,
+            12, 13,
+        ];
+
+        for (i, &dist) in DIST_EXTRA.iter().enumerate() {
+            assert_eq!(dist, num_extra_bits_for_distance_code(i as u8));
+        }
+    }
 }

miniz_oxide/src/inflate/mod.rs

@@ -2,7 +2,6 @@
 
 #[cfg(feature = "with-alloc")]
 use crate::alloc::{boxed::Box, vec, vec::Vec};
-use ::core::usize;
 #[cfg(all(feature = "std", feature = "with-alloc"))]
 use std::error::Error;
 
@@ -123,7 +122,7 @@ fn decompress_error(status: TINFLStatus, output: Vec<u8>) -> Result<Vec<u8>, Dec
 #[inline]
 #[cfg(feature = "with-alloc")]
 pub fn decompress_to_vec(input: &[u8]) -> Result<Vec<u8>, DecompressError> {
-    decompress_to_vec_inner(input, 0, usize::max_value())
+    decompress_to_vec_inner(input, 0, usize::MAX)
 }
 
 /// Decompress the deflate-encoded data (with a zlib wrapper) in `input` to a vector.
@@ -139,7 +138,7 @@ pub fn decompress_to_vec_zlib(input: &[u8]) -> Result<Vec<u8>, DecompressError>
     decompress_to_vec_inner(
         input,
         inflate_flags::TINFL_FLAG_PARSE_ZLIB_HEADER,
-        usize::max_value(),
+        usize::MAX,
     )
 }
 

miniz_oxide/src/inflate/stream.rs

@@ -227,6 +227,9 @@ pub fn inflate(
     if (flush == MZFlush::Finish) && first_call {
         decomp_flags |= inflate_flags::TINFL_FLAG_USING_NON_WRAPPING_OUTPUT_BUF;
 
+        // The caller is indicating that they want to finish the compression and this is the first call with the current stream
+        // so we can simply write directly to the output buffer.
+        // If there is not enough space for all of the decompressed data we will end up with a failure regardless.
         let status = decompress(&mut state.decomp, next_in, next_out, 0, decomp_flags);
         let in_bytes = status.1;
         let out_bytes = status.2;
@@ -435,7 +438,12 @@ mod test {
         let mut part_in = 0;
         let mut part_out = 0;
         for i in 1..=encoded.len() {
-            let res = inflate(&mut state, &encoded[part_in..i], &mut out[part_out..], MZFlush::None);
+            let res = inflate(
+                &mut state,
+                &encoded[part_in..i],
+                &mut out[part_out..],
+                MZFlush::None,
+            );
             let status = res.status.expect("Failed to decompress!");
             if i == encoded.len() {
                 assert_eq!(status, MZStatus::StreamEnd);
@@ -451,7 +459,6 @@ mod test {
         assert_eq!(state.decompressor().adler32(), Some(459605011));
     }
 
-
     // Inflate part of a stream and clone the inflate state.
     // Discard the original state and resume the stream from the clone.
     #[test]
@@ -474,14 +481,21 @@ mod test {
         drop(state);
 
         // Resume the stream using the cloned state
-        let res2 = inflate(&mut resume, &encoded[res1.bytes_consumed..], &mut out[res1.bytes_written..], MZFlush::Finish);
+        let res2 = inflate(
+            &mut resume,
+            &encoded[res1.bytes_consumed..],
+            &mut out[res1.bytes_written..],
+            MZFlush::Finish,
+        );
         let status = res2.status.expect("Failed to decompress!");
         assert_eq!(status, MZStatus::StreamEnd);
 
         assert_eq!(res1.bytes_consumed + res2.bytes_consumed, encoded.len());
         assert_eq!(res1.bytes_written + res2.bytes_written, decoded.len());
-        assert_eq!(&out[..res1.bytes_written + res2.bytes_written as usize], decoded);
+        assert_eq!(
+            &out[..res1.bytes_written + res2.bytes_written as usize],
+            decoded
+        );
         assert_eq!(resume.decompressor().adler32(), Some(459605011));
     }
-
 }

miniz_oxide/tests/test.rs

@@ -250,6 +250,43 @@ fn issue_143_return_buf_error_on_finish_without_end_header() {
     assert_eq!(inflate_result.status.unwrap_err(), MZError::Buf)
 }
 
+/*
+#[test]
+fn partial_decompression_imap_issue_158() {
+    use miniz_oxide::inflate::stream::{inflate, InflateState};
+    use miniz_oxide::{DataFormat, MZFlush};
+    use std::string;
+
+    // Decompresses to
+    // "* QUOTAROOT INBOX \"User quota\"\r\n* QUOTA \"User quota\" (STORAGE 76 307200)\r\nA0001 OK Getquotaroot completed (0.001 + 0.000 secs).\r\n"
+    let input = vec![
+        210, 82, 8, 12, 245, 15, 113, 12, 242, 247, 15, 81, 240, 244, 115, 242, 143, 80, 80, 10,
+        45, 78, 45, 82, 40, 44, 205, 47, 73, 84, 226, 229, 210, 130, 200, 163, 136, 42, 104, 4,
+        135, 248, 7, 57, 186, 187, 42, 152, 155, 41, 24, 27, 152, 27, 25, 24, 104, 242, 114, 57,
+        26, 24, 24, 24, 42, 248, 123, 43, 184, 167, 150, 128, 213, 21, 229, 231, 151, 40, 36, 231,
+        231, 22, 228, 164, 150, 164, 166, 40, 104, 24, 232, 129, 20, 104, 43, 128, 104, 3, 133,
+        226, 212, 228, 98, 77, 61, 94, 46, 0, 0, 0, 0, 255, 255,
+    ];
+
+    let mut inflate_stream = InflateState::new(DataFormat::Raw);
+    let mut output = vec![0; 8];
+    let result = inflate(&mut inflate_stream, &input, &mut output, MZFlush::None);
+
+    let out_string: String = string::String::from_utf8(output).unwrap();
+
+    println!("{}", out_string);
+    println!("written {}", result.bytes_written);
+
+    assert!(result.status.is_ok());
+    // Should not consume everything, there is not enough space in the buffer for the output.
+    assert!(
+        result.bytes_consumed < input.len(),
+        "bytes consumed {:?}, input.len() {}",
+        result.bytes_consumed,
+        input.len()
+    )
+}*/
+
 /*
 #[test]
 fn large_file() {