Skip to content

Commit

Permalink
Minor linting
Browse files Browse the repository at this point in the history
* Optimize/cleanup use statements
* Do not expose `scroll_derive` as a feature in Cargo.toml
* Minor clippy lints

- [ ] There is a FIXME in the code that needs to be addressed before merging
  • Loading branch information
nyurik committed Nov 6, 2023
1 parent b4de465 commit 20597f4
Show file tree
Hide file tree
Showing 9 changed files with 40 additions and 55 deletions.
8 changes: 4 additions & 4 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ description = "A suite of powerful, extensible, generic, endian-aware Read/Write
include = ["src/**/*", "Cargo.toml", "LICENSE", "README.md"]
rust-version = "1.63"

[dependencies]
scroll_derive = { version = "0.11", optional = true, path = "scroll_derive" }

[features]
default = ["std"]
std = []
derive = ["scroll_derive"]
derive = ["dep:scroll_derive"]

[dependencies]
scroll_derive = { version = "0.11", optional = true, path = "scroll_derive" }

[dev-dependencies]
rayon = "1"
Expand Down
2 changes: 1 addition & 1 deletion examples/data_ctx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ impl<'a> ctx::TryFromCtx<'a, Endian> for Data<'a> {
fn try_from_ctx(src: &'a [u8], endian: Endian) -> Result<(Self, usize), Self::Error> {
let name = src.pread::<&'a str>(0)?;
let id = src.pread_with(name.len() + 1, endian)?;
Ok((Data { name: name, id: id }, name.len() + 4))
Ok((Data { name, id }, name.len() + 4))
}
}

Expand Down
18 changes: 5 additions & 13 deletions src/ctx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,12 +180,9 @@
//! }
//! ```

use core::mem::size_of;
use core::mem::transmute;
use core::mem::{size_of, transmute};
use core::ptr::copy_nonoverlapping;
use core::result;
use core::str;

use core::{result, str};
#[cfg(feature = "std")]
use std::ffi::{CStr, CString};

Expand Down Expand Up @@ -240,18 +237,14 @@ impl Default for StrCtx {

impl StrCtx {
pub fn len(&self) -> usize {
match *self {
match self {
StrCtx::Delimiter(_) | StrCtx::DelimiterUntil(_, _) => 1,
StrCtx::Length(_) => 0,
}
}

pub fn is_empty(&self) -> bool {
if let StrCtx::Length(_) = *self {
true
} else {
false
}
matches!(self, StrCtx::Length(_))
}
}

Expand Down Expand Up @@ -867,11 +860,11 @@ impl TryIntoCtx for CString {
// }

#[cfg(test)]
#[cfg(feature = "std")]
mod tests {
use super::*;

#[test]
#[cfg(feature = "std")]
fn parse_a_cstr() {
let src = CString::new("Hello World").unwrap();
let as_bytes = src.as_bytes_with_nul();
Expand All @@ -883,7 +876,6 @@ mod tests {
}

#[test]
#[cfg(feature = "std")]
fn round_trip_a_c_str() {
let src = CString::new("Hello World").unwrap();
let src = src.as_c_str();
Expand Down
5 changes: 1 addition & 4 deletions src/endian.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,6 @@ impl Endian {
}
#[inline]
pub fn is_little(&self) -> bool {
match *self {
LE => true,
_ => false,
}
*self == LE
}
}
14 changes: 6 additions & 8 deletions src/error.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
use core::fmt::{self, Display};
use core::result;

#[cfg(feature = "std")]
use std::error;
#[cfg(feature = "std")]
use std::io;
use std::{error, io};

#[derive(Debug)]
/// A custom Scroll error
Expand All @@ -20,7 +17,8 @@ pub enum Error {
size: usize,
msg: &'static str,
},
/// A custom Scroll error for reporting messages to clients
/// A custom Scroll error for reporting messages to clients.
/// For no-std, use [`Error::BadInput`] with a static string.
#[cfg(feature = "std")]
Custom(String),
/// Returned when IO based errors are encountered
Expand All @@ -31,7 +29,7 @@ pub enum Error {
#[cfg(feature = "std")]
impl error::Error for Error {
fn description(&self) -> &str {
match *self {
match self {
Error::TooBig { .. } => "TooBig",
Error::BadOffset(_) => "BadOffset",
Error::BadInput { .. } => "BadInput",
Expand All @@ -40,7 +38,7 @@ impl error::Error for Error {
}
}
fn cause(&self) -> Option<&dyn error::Error> {
match *self {
match self {
Error::TooBig { .. } => None,
Error::BadOffset(_) => None,
Error::BadInput { .. } => None,
Expand All @@ -59,7 +57,7 @@ impl From<io::Error> for Error {

impl Display for Error {
fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
match *self {
match self {
Error::TooBig { ref size, ref len } => {
write!(fmt, "type is too big ({}) for {}", size, len)
}
Expand Down
9 changes: 4 additions & 5 deletions src/leb128.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
use crate::ctx::TryFromCtx;
use crate::error;
use crate::Pread;
use core::convert::{AsRef, From};
use core::result;
use core::u8;
use core::{result, u8};

use crate::ctx::TryFromCtx;
use crate::{error, Pread};

#[derive(Debug, PartialEq, Copy, Clone)]
/// An unsigned leb128 integer
Expand Down
7 changes: 4 additions & 3 deletions src/lesser.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::ctx::{FromCtx, IntoCtx, SizeWith};
use std::io::{Read, Result, Write};

use crate::ctx::{FromCtx, IntoCtx, SizeWith};

/// An extension trait to `std::io::Read` streams; mainly targeted at reading primitive types with
/// a known size.
///
Expand Down Expand Up @@ -104,8 +105,8 @@ pub trait IOread<Ctx: Copy>: Read {
fn ioread_with<N: FromCtx<Ctx> + SizeWith<Ctx>>(&mut self, ctx: Ctx) -> Result<N> {
let mut scratch = [0u8; 256];
let size = N::size_with(&ctx);
let mut buf = &mut scratch[0..size];
self.read_exact(&mut buf)?;
let buf = &mut scratch[0..size];
self.read_exact(buf)?;
Ok(N::from_ctx(buf, ctx))
}
}
Expand Down
11 changes: 6 additions & 5 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -253,8 +253,7 @@ pub use crate::pwrite::*;

#[doc(hidden)]
pub mod export {
pub use ::core::mem;
pub use ::core::result;
pub use ::core::{mem, result};
}

#[allow(unused)]
Expand All @@ -271,7 +270,9 @@ doc_comment!(include_str!("../README.md"));

#[cfg(test)]
mod tests {
#[allow(overflowing_literals)]
// FIXME: is this needed? It used to be incorrectly declared.
#![allow(overflowing_literals)]

use super::LE;

#[test]
Expand Down Expand Up @@ -462,7 +463,7 @@ mod tests {
fn try_into_ctx(self, this: &mut [u8], le: super::Endian) -> Result<usize, Self::Error> {
use super::Pwrite;
if this.len() < 2 {
return Err((ExternalError {}).into());
return Err(ExternalError {});
}
this.pwrite_with(self.0, 0, le)?;
Ok(2)
Expand All @@ -474,7 +475,7 @@ mod tests {
fn try_from_ctx(this: &'a [u8], le: super::Endian) -> Result<(Self, usize), Self::Error> {
use super::Pread;
if this.len() > 2 {
return Err((ExternalError {}).into());
return Err(ExternalError {});
}
let n = this.pread_with(0, le)?;
Ok((Foo(n), 2))
Expand Down
21 changes: 9 additions & 12 deletions tests/api.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,7 @@
// this exists primarily to test various API usages of scroll; e.g., must compile

// guard against potential undefined behaviour when borrowing from
// packed structs. See https://github.com/rust-lang/rust/issues/46043
#![deny(unaligned_references)]

// #[macro_use] extern crate scroll_derive;
use std::ops::{Deref, DerefMut};

use scroll::ctx::SizeWith;
use scroll::ctx::SizeWith as _;
use scroll::{ctx, Cread, Pread, Result};
use std::ops::{Deref, DerefMut};

#[derive(Default)]
pub struct Section<'a> {
Expand Down Expand Up @@ -87,7 +80,7 @@ pub struct Segment<'a> {

impl<'a> Segment<'a> {
pub fn name(&self) -> Result<&str> {
Ok(self.segname.pread::<&str>(0)?)
self.segname.pread::<&str>(0)
}
pub fn sections(&self) -> Result<Vec<Section<'a>>> {
let nsects = self.nsects as usize;
Expand Down Expand Up @@ -170,13 +163,15 @@ fn lifetime_passthrough() {
assert!(true)
}

#[cfg(feature = "std")]
#[derive(Default)]
#[repr(packed)]
struct Foo {
foo: i64,
bar: u32,
}

#[cfg(feature = "std")]
impl scroll::ctx::FromCtx<scroll::Endian> for Foo {
fn from_ctx(bytes: &[u8], ctx: scroll::Endian) -> Self {
Foo {
Expand All @@ -186,6 +181,7 @@ impl scroll::ctx::FromCtx<scroll::Endian> for Foo {
}
}

#[cfg(feature = "std")]
impl scroll::ctx::SizeWith<scroll::Endian> for Foo {
fn size_with(_: &scroll::Endian) -> usize {
::std::mem::size_of::<Foo>()
Expand All @@ -197,6 +193,7 @@ impl scroll::ctx::SizeWith<scroll::Endian> for Foo {
fn ioread_api() {
use scroll::{IOread, LE};
use std::io::Cursor;

let bytes_ = [
0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xef, 0xbe, 0x00, 0x00,
];
Expand Down Expand Up @@ -261,8 +258,8 @@ fn cread_api_badindex() {

#[test]
fn cwrite_api() {
use scroll::Cread;
use scroll::Cwrite;
use scroll::{Cread, Cwrite};

let mut bytes = [0x0; 16];
bytes.cwrite::<u64>(42, 0);
bytes.cwrite::<u32>(0xdeadbeef, 8);
Expand Down

0 comments on commit 20597f4

Please sign in to comment.