Skip to content

Commit

Permalink
Make Clippy happier, minor cleanups (#283)
Browse files Browse the repository at this point in the history
* Make Clippy happier, minor cleanups

I ran `cargo clippy` and addressed some issues it raised, while leaving other issues for another time. Hopefully this will make this code a bit easier for newcomers to read.

Biggest changes:
* `foo.len() > 0` ➡ `!foo.is_empty()`
* `return x;` ➡ `x`
* `{ foo: foo }` ➡ `{ foo }`
* adding a few `Default` trait implementations
* some `match` statement cleanup

Also, fixed two `ffi::uInt::max_value()` ➡ `ffi::uInt::MAX` due to deprecation.

Biggest TODOs (please give feedback if they should be fixed too):
* Clippy has complained about many `foo >> 0` -- which is a noop that might confuse (?)
* some other corner cases that I don't know enough about

* cargo fmt
  • Loading branch information
nyurik authored Dec 13, 2021
1 parent a5a38c5 commit 0fa0a7b
Show file tree
Hide file tree
Showing 8 changed files with 50 additions and 60 deletions.
2 changes: 1 addition & 1 deletion src/bufreader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ impl<R: Read> BufReader<R> {

pub fn with_buf(buf: Vec<u8>, inner: R) -> BufReader<R> {
BufReader {
inner: inner,
inner,
buf: buf.into_boxed_slice(),
pos: 0,
cap: 0,
Expand Down
6 changes: 6 additions & 0 deletions src/crc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@ pub struct CrcReader<R> {
crc: Crc,
}

impl Default for Crc {
fn default() -> Self {
Self::new()
}
}

impl Crc {
/// Create a new CRC.
pub fn new() -> Crc {
Expand Down
23 changes: 7 additions & 16 deletions src/gz/bufread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ fn copy(into: &mut [u8], from: &[u8], pos: &mut usize) -> usize {
*slot = *val;
}
*pos += min;
return min;
min
}

pub(crate) fn corrupt() -> io::Error {
Expand Down Expand Up @@ -126,15 +126,7 @@ pub(crate) fn read_gz_header<R: Read>(r: &mut R) -> io::Result<GzHeader> {
let mut reader = Buffer::new(&mut part, r);
read_gz_header_part(&mut reader)
};

match result {
Ok(()) => {
return Ok(part.take_header());
}
Err(err) => {
return Err(err);
}
};
result.map(|()| part.take_header())
}

/// A gzip streaming encoder
Expand Down Expand Up @@ -179,7 +171,7 @@ pub fn gz_encoder<R: BufRead>(header: Vec<u8>, r: R, lvl: Compression) -> GzEnco
let crc = CrcReader::new(r);
GzEncoder {
inner: deflate::bufread::DeflateEncoder::new(crc, lvl),
header: header,
header,
pos: 0,
eof: false,
}
Expand Down Expand Up @@ -363,7 +355,7 @@ impl GzHeaderPartial {
}

pub fn take_header(self) -> GzHeader {
return self.header;
self.header
}
}

Expand Down Expand Up @@ -443,7 +435,7 @@ where
self.part.buf.truncate(0);
self.buf_cur = 0;
self.buf_max = 0;
return Ok(rlen);
Ok(rlen)
}
}

Expand Down Expand Up @@ -523,7 +515,7 @@ impl<R: BufRead> Read for GzDecoder<R> {
let mut reader = Buffer::new(&mut part, reader.get_mut().get_mut());
read_gz_header_part(&mut reader)
};
let state = match result {
match result {
Ok(()) => {
*header = Some(part.take_header());
GzState::Body
Expand All @@ -533,8 +525,7 @@ impl<R: BufRead> Read for GzDecoder<R> {
return Err(err);
}
Err(err) => return Err(err),
};
state
}
}
GzState::Body => {
if into.is_empty() {
Expand Down
39 changes: 18 additions & 21 deletions src/gz/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,12 @@ pub struct GzBuilder {
mtime: u32,
}

impl Default for GzBuilder {
fn default() -> Self {
Self::new()
}
}

impl GzBuilder {
/// Create a new blank builder with no header by default.
pub fn new() -> GzBuilder {
Expand Down Expand Up @@ -204,28 +210,19 @@ impl GzBuilder {
} = self;
let mut flg = 0;
let mut header = vec![0u8; 10];
match extra {
Some(v) => {
flg |= FEXTRA;
header.push((v.len() >> 0) as u8);
header.push((v.len() >> 8) as u8);
header.extend(v);
}
None => {}
if let Some(v) = extra {
flg |= FEXTRA;
header.push((v.len() >> 0) as u8);
header.push((v.len() >> 8) as u8);
header.extend(v);
}
match filename {
Some(filename) => {
flg |= FNAME;
header.extend(filename.as_bytes_with_nul().iter().map(|x| *x));
}
None => {}
if let Some(filename) = filename {
flg |= FNAME;
header.extend(filename.as_bytes_with_nul().iter().map(|x| *x));
}
match comment {
Some(comment) => {
flg |= FCOMMENT;
header.extend(comment.as_bytes_with_nul().iter().map(|x| *x));
}
None => {}
if let Some(comment) = comment {
flg |= FCOMMENT;
header.extend(comment.as_bytes_with_nul().iter().map(|x| *x));
}
header[0] = 0x1f;
header[1] = 0x8b;
Expand All @@ -248,7 +245,7 @@ impl GzBuilder {
// default this value to 255. I'm not sure that if we "correctly" set
// this it'd do anything anyway...
header[9] = operating_system.unwrap_or(255);
return header;
header
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/gz/read.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ pub struct GzEncoder<R> {
}

pub fn gz_encoder<R: Read>(inner: bufread::GzEncoder<BufReader<R>>) -> GzEncoder<R> {
GzEncoder { inner: inner }
GzEncoder { inner }
}

impl<R: Read> GzEncoder<R> {
Expand Down
16 changes: 7 additions & 9 deletions src/gz/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ pub fn gz_encoder<W: Write>(header: Vec<u8>, w: W, lvl: Compression) -> GzEncode
GzEncoder {
inner: zio::Writer::new(w, Compress::new(lvl, false)),
crc: Crc::new(),
header: header,
header,
crc_bytes_written: 0,
}
}
Expand Down Expand Up @@ -134,7 +134,7 @@ impl<W: Write> GzEncoder<W> {
}

fn write_header(&mut self) -> io::Result<()> {
while self.header.len() > 0 {
while !self.header.is_empty() {
let n = self.inner.get_mut().write(&self.header)?;
self.header.drain(..n);
}
Expand Down Expand Up @@ -368,13 +368,11 @@ impl<W: Write> Write for GzDecoder<W> {
} else {
let (n, status) = self.inner.write_with_status(buf)?;

if status == Status::StreamEnd {
if n < buf.len() && self.crc_bytes.len() < 8 {
let remaining = buf.len() - n;
let crc_bytes = cmp::min(remaining, CRC_BYTES_LEN - self.crc_bytes.len());
self.crc_bytes.extend(&buf[n..n + crc_bytes]);
return Ok(n + crc_bytes);
}
if status == Status::StreamEnd && n < buf.len() && self.crc_bytes.len() < 8 {
let remaining = buf.len() - n;
let crc_bytes = cmp::min(remaining, CRC_BYTES_LEN - self.crc_bytes.len());
self.crc_bytes.extend(&buf[n..n + crc_bytes]);
return Ok(n + crc_bytes);
}
Ok(n)
}
Expand Down
8 changes: 4 additions & 4 deletions src/mem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ impl Compress {
let stream = &mut *self.inner.inner.stream_wrapper;
stream.msg = std::ptr::null_mut();
let rc = unsafe {
assert!(dictionary.len() < ffi::uInt::max_value() as usize);
assert!(dictionary.len() < ffi::uInt::MAX as usize);
ffi::deflateSetDictionary(stream, dictionary.as_ptr(), dictionary.len() as ffi::uInt)
};

Expand Down Expand Up @@ -367,7 +367,7 @@ impl Compress {
self.compress(input, out, flush)
};
output.set_len((self.total_out() - before) as usize + len);
return ret;
ret
}
}
}
Expand Down Expand Up @@ -508,7 +508,7 @@ impl Decompress {
self.decompress(input, out, flush)
};
output.set_len((self.total_out() - before) as usize + len);
return ret;
ret
}
}

Expand All @@ -518,7 +518,7 @@ impl Decompress {
let stream = &mut *self.inner.inner.stream_wrapper;
stream.msg = std::ptr::null_mut();
let rc = unsafe {
assert!(dictionary.len() < ffi::uInt::max_value() as usize);
assert!(dictionary.len() < ffi::uInt::MAX as usize);
ffi::inflateSetDictionary(stream, dictionary.as_ptr(), dictionary.len() as ffi::uInt)
};

Expand Down
14 changes: 6 additions & 8 deletions src/zio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,9 @@ where
// then we need to keep asking for more data because if we
// return that 0 bytes of data have been read then it will
// be interpreted as EOF.
Ok(Status::Ok) | Ok(Status::BufError) if read == 0 && !eof && dst.len() > 0 => continue,
Ok(Status::Ok) | Ok(Status::BufError) if read == 0 && !eof && !dst.is_empty() => {
continue
}
Ok(Status::Ok) | Ok(Status::BufError) | Ok(Status::StreamEnd) => return Ok(read),

Err(..) => {
Expand Down Expand Up @@ -216,13 +218,9 @@ impl<W: Write, D: Ops> Writer<W, D> {
let before_in = self.data.total_in();
let ret = self.data.run_vec(buf, &mut self.buf, D::Flush::none());
let written = (self.data.total_in() - before_in) as usize;
let is_stream_end = matches!(ret, Ok(Status::StreamEnd));

let is_stream_end = match ret {
Ok(Status::StreamEnd) => true,
_ => false,
};

if buf.len() > 0 && written == 0 && ret.is_ok() && !is_stream_end {
if !buf.is_empty() && written == 0 && ret.is_ok() && !is_stream_end {
continue;
}
return match ret {
Expand All @@ -240,7 +238,7 @@ impl<W: Write, D: Ops> Writer<W, D> {
fn dump(&mut self) -> io::Result<()> {
// TODO: should manage this buffer not with `drain` but probably more of
// a deque-like strategy.
while self.buf.len() > 0 {
while !self.buf.is_empty() {
let n = self.obj.as_mut().unwrap().write(&self.buf)?;
if n == 0 {
return Err(io::ErrorKind::WriteZero.into());
Expand Down

0 comments on commit 0fa0a7b

Please sign in to comment.