Skip to content
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

Add nostd tests #89

Merged
merged 3 commits into from
Nov 6, 2023
Merged
Show file tree
Hide file tree
Changes from 2 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
1 change: 1 addition & 0 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,4 @@ jobs:
cargo fmt -- --check
- run: cargo build
- run: cargo test
- run: cargo test --no-default-features
4 changes: 4 additions & 0 deletions src/ctx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@ pub trait FromCtx<Ctx: Copy = (), This: ?Sized = [u8]> {
/// `[u8]`), then you need to implement this trait
///
/// ```rust
/// ##[cfg(feature = "std")] {
/// use scroll::{self, ctx, Pread};
/// #[derive(Debug, PartialEq, Eq)]
/// pub struct Foo(u16);
Expand All @@ -286,6 +287,7 @@ pub trait FromCtx<Ctx: Copy = (), This: ?Sized = [u8]> {
///
/// let foo2 = bytes.pread_with::<Foo>(0, scroll::BE).unwrap();
/// assert_eq!(Foo(0xdeadu16), foo2);
/// # }
/// ```
///
/// # Advanced: Using Your Own Error in `TryFromCtx`
Expand Down Expand Up @@ -350,6 +352,7 @@ pub trait IntoCtx<Ctx: Copy = (), This: ?Sized = [u8]>: Sized {
/// To implement writing into an arbitrary byte buffer, implement `TryIntoCtx`
/// # Example
/// ```rust
/// ##[cfg(feature = "std")] {
/// use scroll::{self, ctx, LE, Endian, Pwrite};
/// #[derive(Debug, PartialEq, Eq)]
/// pub struct Foo(u16);
Expand All @@ -369,6 +372,7 @@ pub trait IntoCtx<Ctx: Copy = (), This: ?Sized = [u8]>: Sized {
///
/// let mut bytes: [u8; 4] = [0, 0, 0, 0];
/// bytes.pwrite_with(Foo(0x7f), 1, LE).unwrap();
/// # }
/// ```
pub trait TryIntoCtx<Ctx: Copy = (), This: ?Sized = [u8]>: Sized {
type Error;
Expand Down
11 changes: 9 additions & 2 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,13 @@ pub enum Error {
size: usize,
msg: &'static str,
},
#[cfg(feature = "std")]
/// A custom Scroll error for reporting messages to clients
Custom(String),
#[cfg(feature = "std")]
Custom(String),
/// A custom static Scroll error for reporting messages to clients
CustomStatic(&'static str),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok sorry to go back and forth, but can you remove these changes (adding CustomStatic); i thought it would be required to get the tests to pass, but a change like this (which is a breaking change) would require clients to update on the additional matches. Because this is just for fixing tests I don't think it should be included in this PR.

Additionally, it's unclear to me why we need this, since technically in a no-std environment the user of scroll could return BadInput which also has a static str (they could set size to 0 if they wanted).

Once the static str stuff is walked out we can merge this immediately, sorry for the busy work and appreciate you getting these changes in, but adding a new variant should be discussed separately I think (and isn't required for this, so let's remove it)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, np, removed

/// Returned when IO based errors are encountered
#[cfg(feature = "std")]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i just realized this got added; this is technically a breaking change, but nostd could never have worked with this, iiuc?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correct, io does not exist without std, so technically it is not a breaking change :)

IO(io::Error),
}

Expand All @@ -36,6 +38,7 @@ impl error::Error for Error {
Error::BadOffset(_) => "BadOffset",
Error::BadInput { .. } => "BadInput",
Error::Custom(_) => "Custom",
Error::CustomStatic(_) => "Custom", // Both Custom and CustomStatic are custom errors
Error::IO(_) => "IO",
}
}
Expand All @@ -45,6 +48,7 @@ impl error::Error for Error {
Error::BadOffset(_) => None,
Error::BadInput { .. } => None,
Error::Custom(_) => None,
Error::CustomStatic(_) => None,
Error::IO(ref io) => io.source(),
}
}
Expand Down Expand Up @@ -73,6 +77,9 @@ impl Display for Error {
Error::Custom(ref msg) => {
write!(fmt, "{}", msg)
}
Error::CustomStatic(msg) => {
write!(fmt, "{msg}")
}
#[cfg(feature = "std")]
Error::IO(ref err) => {
write!(fmt, "{}", err)
Expand Down
3 changes: 3 additions & 0 deletions src/leb128.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,20 +184,23 @@ mod tests {
let buf = [2u8 | CONTINUATION_BIT, 1];
let bytes = &buf[..];
let num = bytes.pread::<Uleb128>(0).unwrap();
#[cfg(feature = "std")]
println!("num: {:?}", &num);
assert_eq!(130u64, num.into());
assert_eq!(num.size(), 2);

let buf = [0x00, 0x01];
let bytes = &buf[..];
let num = bytes.pread::<Uleb128>(0).unwrap();
#[cfg(feature = "std")]
println!("num: {:?}", &num);
assert_eq!(0u64, num.into());
assert_eq!(num.size(), 1);

let buf = [0x21];
let bytes = &buf[..];
let num = bytes.pread::<Uleb128>(0).unwrap();
#[cfg(feature = "std")]
println!("num: {:?}", &num);
assert_eq!(0x21u64, num.into());
assert_eq!(num.size(), 1);
Expand Down
38 changes: 25 additions & 13 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@
//! [FromCtx](ctx/trait.FromCtx.html) and [SizeWith](ctx/trait.SizeWith.html).
//!
//! ```rust
//! ##[cfg(feature = "std")] {
//! use std::io::Cursor;
//! use scroll::{IOread, ctx, Endian};
//! let bytes = [0x01,0x00,0x00,0x00,0x00,0x00,0x00,0x00, 0xef,0xbe,0x00,0x00,];
Expand All @@ -139,12 +140,14 @@
//! // read/written, e.g. switching between ELF32 or ELF64 at runtime.
//! let size = <u64 as ctx::SizeWith<Endian>>::size_with(&Endian::Little) as u64;
//! assert_eq!(prev + size, after);
//! # }
//! ```
//!
//! In the same vein as IOread we can use IOwrite to write a type to anything implementing
//! `std::io::Write`:
//!
//! ```rust
//! ##[cfg(feature = "std")] {
//! use std::io::Cursor;
//! use scroll::{IOwrite};
//!
Expand All @@ -155,6 +158,7 @@
//! cursor.iowrite_with(0xdeadbeef as u32, scroll::BE).unwrap();
//!
//! assert_eq!(cursor.into_inner(), [0xde, 0xad, 0xbe, 0xef, 0x0]);
//! # }
//! ```
//!
//! ## Complex use cases
Expand Down Expand Up @@ -355,18 +359,22 @@ mod tests {
let bytes: [u8; 2] = [0x2e, 0x0];
let b = &bytes[..];
let s: &str = b.pread(0).unwrap();
#[cfg(feature = "std")]
println!("str: {}", s);
assert_eq!(s.len(), bytes[..].len() - 1);
let bytes: &[u8] = b"hello, world!\0some_other_things";
let hello_world: &str = bytes.pread_with(0, StrCtx::Delimiter(NULL)).unwrap();
#[cfg(feature = "std")]
println!("{:?}", &hello_world);
assert_eq!(hello_world.len(), 13);
let hello: &str = bytes.pread_with(0, StrCtx::Delimiter(SPACE)).unwrap();
#[cfg(feature = "std")]
println!("{:?}", &hello);
assert_eq!(hello.len(), 6);
// this could result in underflow so we just try it
let _error = bytes.pread_with::<&str>(6, StrCtx::Delimiter(SPACE));
let error = bytes.pread_with::<&str>(7, StrCtx::Delimiter(SPACE));
#[cfg(feature = "std")]
println!("{:?}", &error);
assert!(error.is_ok());
}
Expand All @@ -377,13 +385,16 @@ mod tests {
use super::Pread;
let bytes: &[u8] = b"";
let hello_world = bytes.pread_with::<&str>(0, StrCtx::Delimiter(NULL));
#[cfg(feature = "std")]
println!("1 {:?}", &hello_world);
assert_eq!(hello_world.is_err(), true);
let error = bytes.pread_with::<&str>(7, StrCtx::Delimiter(SPACE));
#[cfg(feature = "std")]
println!("2 {:?}", &error);
assert!(error.is_err());
let bytes: &[u8] = b"\0";
let null = bytes.pread::<&str>(0).unwrap();
#[cfg(feature = "std")]
println!("3 {:?}", &null);
assert_eq!(null.len(), 0);
}
Expand Down Expand Up @@ -413,8 +424,7 @@ mod tests {
assert_eq!(bytes, "bytes");
}

use std::error;
use std::fmt::{self, Display};
use core::fmt::{self, Display};

#[derive(Debug)]
pub struct ExternalError {}
Expand All @@ -425,11 +435,12 @@ mod tests {
}
}

impl error::Error for ExternalError {
#[cfg(feature = "std")]
impl std::error::Error for ExternalError {
fn description(&self) -> &str {
"ExternalError"
}
fn cause(&self) -> Option<&dyn error::Error> {
fn cause(&self) -> Option<&dyn std::error::Error> {
None
}
}
Expand Down Expand Up @@ -499,7 +510,7 @@ mod tests {
let mut offset = 0;
let deadbeef: $typ = bytes.gread_with(&mut offset, LE).unwrap();
assert_eq!(deadbeef, $deadbeef as $typ);
assert_eq!(offset, ::std::mem::size_of::<$typ>());
assert_eq!(offset, ::core::mem::size_of::<$typ>());
}
};
}
Expand All @@ -518,7 +529,7 @@ mod tests {
let mut offset = 0;
let deadbeef: $typ = bytes.gread_with(&mut offset, LE).unwrap();
assert_eq!(deadbeef, $deadbeef as $typ);
assert_eq!(offset, ::std::mem::size_of::<$typ>());
assert_eq!(offset, ::core::mem::size_of::<$typ>());
}
};
}
Expand All @@ -537,8 +548,8 @@ mod tests {
let o2 = &mut 0;
let val: $typ = buffer.gread_with(o2, LE).unwrap();
assert_eq!(val, $val);
assert_eq!(*offset, ::std::mem::size_of::<$typ>());
assert_eq!(*o2, ::std::mem::size_of::<$typ>());
assert_eq!(*offset, ::core::mem::size_of::<$typ>());
assert_eq!(*o2, ::core::mem::size_of::<$typ>());
assert_eq!(*o2, *offset);
buffer.gwrite_with($val.clone(), offset, BE).unwrap();
let val: $typ = buffer.gread_with(o2, BE).unwrap();
Expand Down Expand Up @@ -612,16 +623,17 @@ mod tests {
let res = b.gread_with::<&str>(offset, StrCtx::Length(3));
assert!(res.is_err());
*offset = 0;
let astring: [u8; 3] = [0x45, 042, 0x44];
let astring: [u8; 3] = [0x45, 0x42, 0x44];
let string = astring.gread_with::<&str>(offset, StrCtx::Length(2));
match &string {
&Ok(_) => {}
&Err(ref err) => {
println!("{}", &err);
Ok(_) => {}
Err(_err) => {
#[cfg(feature = "std")]
println!("{_err}");
panic!();
}
}
assert_eq!(string.unwrap(), "E*");
assert_eq!(string.unwrap(), "EB");
*offset = 0;
let bytes2: &[u8] = b.gread_with(offset, 2).unwrap();
assert_eq!(*offset, 2);
Expand Down
1 change: 1 addition & 0 deletions tests/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ impl scroll::ctx::SizeWith<scroll::Endian> for Foo {
}

#[test]
#[cfg(feature = "std")]
fn ioread_api() {
use scroll::{IOread, LE};
use std::io::Cursor;
Expand Down
Loading