Skip to content

Fix some pedantic clippy lints and improve code style and small allocations #82

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions benches/decode_all.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ use ruzstd::decoding::FrameDecoder;

fn criterion_benchmark(c: &mut Criterion) {
let mut fr = FrameDecoder::new();
let mut target_slice = &mut vec![0u8; 1024 * 1024 * 200];
let target_slice = &mut vec![0u8; 1024 * 1024 * 200];
let src = include_bytes!("../decodecorpus_files/z000033.zst");

c.bench_function("decode_all_slice", |b| {
b.iter(|| {
fr.decode_all(src, &mut target_slice).unwrap();
fr.decode_all(src, target_slice).unwrap();
})
});
}
Expand Down
4 changes: 2 additions & 2 deletions src/bin/zstd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ fn decompress(flags: &[String], file_paths: &[String]) {
})) => {
eprintln!("Found a skippable frame with magic number: {magic_num} and size: {skip_size}");
tracker.file_pos = f.stream_position().unwrap();
tracker.file_pos += skip_size as u64;
f.seek(SeekFrom::Current(skip_size as i64)).unwrap();
tracker.file_pos += u64::from(skip_size);
f.seek(SeekFrom::Current(i64::from(skip_size))).unwrap();
continue;
}
other => other.unwrap(),
Expand Down
4 changes: 2 additions & 2 deletions src/blocks/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@ pub struct BlockHeader {
pub last_block: bool,
pub block_type: BlockType,
/// The size of the decompressed data. If the block type
/// is [BlockType::Reserved] or [BlockType::Compressed],
/// is [`BlockType::Reserved`] or [`BlockType::Compressed`],
/// this value is set to zero and should not be referenced.
pub decompressed_size: u32,
/// The size of the block. If the block is [BlockType::RLE],
/// The size of the block. If the block is [`BlockType::RLE`],
/// this value will be 1.
pub content_size: u32,
}
26 changes: 13 additions & 13 deletions src/blocks/literals_section.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,20 @@ use crate::decoding::errors::LiteralsSectionParseError;
///
/// This is the first of those two sections. A literal is just any arbitrary data, and it is copied by the sequences section
pub struct LiteralsSection {
/// - If this block is of type [LiteralsSectionType::Raw], then the data is `regenerated_bytes`
/// - If this block is of type [`LiteralsSectionType::Raw`], then the data is `regenerated_bytes`
/// bytes long, and it contains the raw literals data to be used during the second section,
/// the sequences section.
/// - If this block is of type [LiteralsSectionType::RLE],
/// - If this block is of type [`LiteralsSectionType::RLE`],
/// then the literal consists of a single byte repeated `regenerated_size` times.
/// - For types [LiteralsSectionType::Compressed] or [LiteralsSectionType::Treeless],
/// - For types [`LiteralsSectionType::Compressed`] or [`LiteralsSectionType::Treeless`],
/// then this is the size of the decompressed data.
pub regenerated_size: u32,
/// - For types [LiteralsSectionType::Raw] and [LiteralsSectionType::RLE], this value is not present.
/// - For types [LiteralsSectionType::Compressed] and [LiteralsSectionType::Treeless], this value will
/// - For types [`LiteralsSectionType::Raw`] and [`LiteralsSectionType::RLE`], this value is not present.
/// - For types [`LiteralsSectionType::Compressed`] and [`LiteralsSectionType::Treeless`], this value will
/// be set to the size of the compressed data.
pub compressed_size: Option<u32>,
/// This value will be either 1 stream or 4 streams if the literal is of type
/// [LiteralsSectionType::Compressed] or [LiteralsSectionType::Treeless], and it
/// [`LiteralsSectionType::Compressed`] or [`LiteralsSectionType::Treeless`], and it
/// is not used for RLE or uncompressed literals.
pub num_streams: Option<u8>,
/// The type of the literal section.
Expand All @@ -31,15 +31,15 @@ pub struct LiteralsSection {
pub enum LiteralsSectionType {
/// Literals are stored uncompressed.
Raw,
/// Literals consist of a single byte value repeated [LiteralsSection::regenerated_size] times.
/// Literals consist of a single byte value repeated [`LiteralsSection::regenerated_size`] times.
#[allow(clippy::upper_case_acronyms)]
RLE,
/// This is a standard Huffman-compressed block, starting with a Huffman tree description.
/// In this mode, there are at least *2* different literals represented in the Huffman tree
/// description.
Compressed,
/// This is a Huffman-compressed block,
/// using the Huffman tree from the previous [LiteralsSectionType::Compressed] block
/// using the Huffman tree from the previous [`LiteralsSectionType::Compressed`] block
/// in the sequence. If this mode is triggered without any previous Huffman-tables in the
/// frame (or dictionary), it should be treated as data corruption.
Treeless,
Expand All @@ -52,7 +52,7 @@ impl Default for LiteralsSection {
}

impl LiteralsSection {
/// Create a new [LiteralsSection].
/// Create a new [`LiteralsSection`].
pub fn new() -> LiteralsSection {
LiteralsSection {
regenerated_size: 0,
Expand All @@ -63,7 +63,7 @@ impl LiteralsSection {
}

/// Given the first byte of a header, determine the size of the whole header, from 1 to 5 bytes.
pub fn header_bytes_needed(&self, first_byte: u8) -> Result<u8, LiteralsSectionParseError> {
pub fn header_bytes_needed(first_byte: u8) -> Result<u8, LiteralsSectionParseError> {
let ls_type: LiteralsSectionType = Self::section_type(first_byte)?;
let size_format = (first_byte >> 2) & 0x3;
match ls_type {
Expand All @@ -84,7 +84,7 @@ impl LiteralsSection {
// regenerated_size uses 20 bits
Ok(3)
}
_ => panic!(
_ => unreachable!(
"This is a bug in the program. There should only be values between 0..3"
),
}
Expand All @@ -105,7 +105,7 @@ impl LiteralsSection {
Ok(5)
}

_ => panic!(
_ => unreachable!(
"This is a bug in the program. There should only be values between 0..3"
),
}
Expand All @@ -120,7 +120,7 @@ impl LiteralsSection {
self.ls_type = Self::section_type(block_type)?;
let size_format = br.get_bits(2)? as u8;

let byte_needed = self.header_bytes_needed(raw[0])?;
let byte_needed = Self::header_bytes_needed(raw[0])?;
if raw.len() < byte_needed as usize {
return Err(LiteralsSectionParseError::NotEnoughBytes {
have: raw.len(),
Expand Down
4 changes: 2 additions & 2 deletions src/blocks/sequence_section.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ pub enum ModeType {
}

impl CompressionModes {
/// Deserialize a two bit mode value into a [ModeType]
/// Deserialize a two bit mode value into a [`ModeType`]
pub fn decode_mode(m: u8) -> ModeType {
match m {
0 => ModeType::Predefined,
Expand Down Expand Up @@ -96,7 +96,7 @@ impl Default for SequencesHeader {
}

impl SequencesHeader {
/// Create a new [SequencesHeader].
/// Create a new [`SequencesHeader`].
pub fn new() -> SequencesHeader {
SequencesHeader {
num_sequences: 0,
Expand Down
4 changes: 1 addition & 3 deletions src/decoding/bit_reader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,7 @@ impl<'s> BitReader<'s> {
}

pub fn return_bits(&mut self, n: usize) {
if n > self.idx {
panic!("Cant return this many bits");
}
assert!(n <= self.idx, "can't return this many bits");
self.idx -= n;
}

Expand Down
2 changes: 1 addition & 1 deletion src/decoding/bit_reader_reverse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ impl<'s> BitReaderReversed<'s> {
debug_assert!(self.bits_consumed <= 64);
}

/// Same as calling get_bits three times but slightly more performant
/// Same as calling `get_bits` three times but slightly more performant
#[inline(always)]
pub fn get_bits_triple(&mut self, n1: u8, n2: u8, n3: u8) -> (u64, u64, u64) {
let sum = n1 + n2 + n3;
Expand Down
5 changes: 2 additions & 3 deletions src/decoding/block_decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ enum DecoderState {
Failed, //TODO put "self.internal_state = DecoderState::Failed;" everywhere an unresolvable error occurs
}

/// Create a new [BlockDecoder].
/// Create a new [`BlockDecoder`].
pub fn new() -> BlockDecoder {
BlockDecoder {
internal_state: DecoderState::ReadyToDecodeNextHeader,
Expand Down Expand Up @@ -113,7 +113,7 @@ impl BlockDecoder {
}

BlockType::Compressed => {
self.decompress_block(header, workspace, source)?;
Self::decompress_block(header, workspace, source)?;

self.internal_state = DecoderState::ReadyToDecodeNextHeader;
Ok(u64::from(header.content_size))
Expand All @@ -122,7 +122,6 @@ impl BlockDecoder {
}

fn decompress_block(
&mut self,
header: &BlockHeader,
workspace: &mut DecoderScratch, //reuse this as often as possible. Not only if the trees are reused but also reuse the allocations when building new trees
mut source: impl Read,
Expand Down
8 changes: 4 additions & 4 deletions src/decoding/decodebuffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,8 @@ impl DecodeBuffer {
self.buffer.len()
}

/// Drain as much as possible while retaining enough so that decoding si still possible with the required window_size
/// At best call only if can_drain_to_window_size reports a 'high' number of bytes to reduce allocations
/// Drain as much as possible while retaining enough so that decoding si still possible with the required `window_size`
/// At best call only if `can_drain_to_window_size` reports a 'high' number of bytes to reduce allocations
pub fn drain_to_window_size(&mut self) -> Option<Vec<u8>> {
//TODO investigate if it is possible to return the std::vec::Drain iterator directly without collecting here
match self.can_drain_to_window_size() {
Expand Down Expand Up @@ -238,7 +238,7 @@ impl DecodeBuffer {
Ok(amount)
}

/// Semantics of write_bytes:
/// Semantics of `write_bytes`:
/// Should dump as many of the provided bytes as possible to whatever sink until no bytes are left or an error is encountered
/// Return how many bytes have actually been dumped to the sink.
fn drain_to(
Expand Down Expand Up @@ -302,7 +302,7 @@ impl DecodeBuffer {
}
}

/// Like Write::write_all but returns partial write length even on error
/// Like `Write::write_all` but returns partial write length even on error
fn write_all_bytes(mut sink: impl Write, buf: &[u8]) -> (usize, Result<(), Error>) {
let mut written = 0;
while written < buf.len() {
Expand Down
8 changes: 4 additions & 4 deletions src/decoding/dictionary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ pub struct Dictionary {
/// to compress or decompress,
/// so it can be referenced in sequence commands.
/// As long as the amount of data decoded from this frame is less than or
/// equal to Window_Size, sequence commands may specify offsets longer than
/// equal to `Window_Size`, sequence commands may specify offsets longer than
/// the total length of decoded output so far to reference back to the
/// dictionary, even parts of the dictionary with offsets larger than Window_Size.
/// After the total output has surpassed Window_Size however,
/// dictionary, even parts of the dictionary with offsets larger than `Window_Size`.
/// After the total output has surpassed `Window_Size` however,
/// this is no longer allowed and the dictionary is no longer accessible
pub dict_content: Vec<u8>,
/// The 3 most recent offsets are stored so that they can be used
Expand All @@ -41,7 +41,7 @@ pub const MAGIC_NUM: [u8; 4] = [0x37, 0xA4, 0x30, 0xEC];

impl Dictionary {
/// Parses the dictionary from `raw` and set the tables
/// it returns the dict_id for checking with the frame's `dict_id``
/// it returns the `dict_id` for checking with the frame's `dict_id`
pub fn decode_dict(raw: &[u8]) -> Result<Dictionary, DictionaryDecodeError> {
let mut new_dict = Dictionary {
id: 0,
Expand Down
6 changes: 3 additions & 3 deletions src/decoding/frame.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ impl FrameDescriptor {
/// This is a 2 bit flag, specifying if the `Frame_Content_Size` field is present
/// within the header. It notates the number of bytes used by `Frame_Content_size`
///
/// When this value is is 0, `FCS_Field_Size` depends on Single_Segment_flag.
/// When this value is is 0, `FCS_Field_Size` depends on `Single_Segment_flag`.
/// If the `Single_Segment_flag` field is set in the frame header descriptor,
/// the size of the `Frame_Content_Size` field of the header is 1 byte.
/// Otherwise, `FCS_Field_Size` is 0, and the `Frame_Content_Size` is not provided.
Expand Down Expand Up @@ -230,7 +230,7 @@ pub fn read_frame_header(mut r: impl Read) -> Result<(Frame, u8), ReadFrameHeade

#[allow(clippy::needless_range_loop)]
for i in 0..dict_id_len {
dict_id += (buf[i] as u32) << (8 * i);
dict_id += u32::from(buf[i]) << (8 * i);
}
if dict_id != 0 {
frame_header.dict_id = Some(dict_id);
Expand All @@ -248,7 +248,7 @@ pub fn read_frame_header(mut r: impl Read) -> Result<(Frame, u8), ReadFrameHeade

#[allow(clippy::needless_range_loop)]
for i in 0..fcs_len {
fcs += (fcs_buf[i] as u64) << (8 * i);
fcs += u64::from(fcs_buf[i]) << (8 * i);
}
if fcs_len == 2 {
fcs += 256;
Expand Down
Loading
Loading