From ba06d8b8cecc5bacc58a17e3495961e7076a9050 Mon Sep 17 00:00:00 2001 From: wcampbell Date: Wed, 2 Aug 2023 20:21:03 -0400 Subject: [PATCH] wip --- deku-derive/src/lib.rs | 7 +-- deku-derive/src/macros/deku_read.rs | 29 +++++------ examples/custom_reader_and_writer.rs | 13 ++--- examples/example.rs | 1 + src/container.rs | 51 +++++++++---------- src/error.rs | 2 + src/impls/primitive.rs | 20 +++++--- src/impls/vec.rs | 2 +- src/prelude.rs | 4 +- tests/test_attributes/test_ctx.rs | 25 ++++----- .../test_compile/cases/internal_variables.rs | 12 ++--- 11 files changed, 85 insertions(+), 81 deletions(-) diff --git a/deku-derive/src/lib.rs b/deku-derive/src/lib.rs index 402663ca..e1fc07ff 100644 --- a/deku-derive/src/lib.rs +++ b/deku-derive/src/lib.rs @@ -667,10 +667,11 @@ fn apply_replacements(input: &syn::LitStr) -> Result, Repla } let input_str = input_value - .replace("deku::input", "__deku_input") // part of the public API `from_bytes` - .replace("deku::input_bits", "__deku_input_bits") // part of the public API `read` + // TODO: remove these? + //.replace("deku::input", "__deku_input") // part of the public API `from_bytes` + //.replace("deku::input_bits", "__deku_input_bits") // part of the public API `read` .replace("deku::output", "__deku_output") // part of the public API `write` - .replace("deku::rest", "&__deku_rest[__deku_total_read..]") + .replace("deku::reader", "container") .replace("deku::bit_offset", "__deku_bit_offset") .replace("deku::byte_offset", "__deku_byte_offset"); diff --git a/deku-derive/src/macros/deku_read.rs b/deku-derive/src/macros/deku_read.rs index 0d8b10b6..7d788dfc 100644 --- a/deku-derive/src/macros/deku_read.rs +++ b/deku-derive/src/macros/deku_read.rs @@ -63,7 +63,7 @@ fn emit_struct(input: &DekuData) -> Result { quote! { use core::convert::TryFrom; let container = &mut deku::container::Container::new(__deku_input.0); - let _ = container.read_bits(__deku_input.1); + let _ = container.read_bits(__deku_input.1)?; #magic_read @@ -104,7 +104,7 @@ fn emit_struct(input: &DekuData) -> Result { tokens.extend(quote! { impl #imp ::#crate_::DekuRead<#lifetime, #ctx_types> for #ident #wher { - fn from_reader(container: &#lifetime mut ::#crate_::container::Container, #ctx_arg) -> core::result::Result { + fn from_reader(container: &mut ::#crate_::container::Container, #ctx_arg) -> core::result::Result { #read_body } } @@ -115,7 +115,7 @@ fn emit_struct(input: &DekuData) -> Result { tokens.extend(quote! { impl #imp ::#crate_::DekuRead<#lifetime> for #ident #wher { - fn from_reader(container: &#lifetime mut ::#crate_::container::Container, _: ()) -> core::result::Result { + fn from_reader(container: &mut ::#crate_::container::Container, _: ()) -> core::result::Result { #read_body } } @@ -274,7 +274,7 @@ fn emit_enum(input: &DekuData) -> Result { let variant_id_read = if id.is_some() { quote! { - let (__deku_amt_read, __deku_variant_id) = (0, (#id)); + let __deku_variant_id = (#id); } } else if id_type.is_some() { quote! { @@ -301,7 +301,7 @@ fn emit_enum(input: &DekuData) -> Result { quote! { use core::convert::TryFrom; let container = &mut deku::container::Container::new(__deku_input.0); - let _ = container.read_bits(__deku_input.1); + let _ = container.read_bits(__deku_input.1)?; #magic_read @@ -340,7 +340,7 @@ fn emit_enum(input: &DekuData) -> Result { tokens.extend(quote! { #[allow(non_snake_case)] impl #imp ::#crate_::DekuRead<#lifetime, #ctx_types> for #ident #wher { - fn from_reader(container: &#lifetime mut ::#crate_::container::Container, #ctx_arg) -> core::result::Result { + fn from_reader(container: &mut ::#crate_::container::Container, #ctx_arg) -> core::result::Result { #read_body } } @@ -352,7 +352,7 @@ fn emit_enum(input: &DekuData) -> Result { tokens.extend(quote! { #[allow(non_snake_case)] impl #imp ::#crate_::DekuRead<#lifetime> for #ident #wher { - fn from_reader(container: &#lifetime mut ::#crate_::container::Container, _: ()) -> core::result::Result { + fn from_reader(container: &mut ::#crate_::container::Container, _: ()) -> core::result::Result { #read_body } } @@ -439,7 +439,7 @@ fn emit_bit_byte_offsets( .any(|v| token_contains_string(v, "__deku_byte_offset")) { Some(quote! { - let __deku_byte_offset = __deku_bit_offset / 8; + let __deku_byte_offset = container.bits_read / 8; }) } else { None @@ -451,7 +451,7 @@ fn emit_bit_byte_offsets( || byte_offset.is_some() { Some(quote! { - let __deku_bit_offset = usize::try_from(unsafe { __deku_rest.as_bitptr().offset_from(__deku_input_bits.as_bitptr()) } )?; + let __deku_bit_offset = container.bits_read; }) } else { None @@ -465,6 +465,7 @@ fn emit_padding(bit_size: &TokenStream) -> TokenStream { quote! { { use core::convert::TryFrom; + // TODO: I hope this consts in most cases? let __deku_pad = usize::try_from(#bit_size).map_err(|e| ::#crate_::DekuError::InvalidParam(format!( "Invalid padding param \"({})\": cannot convert to usize", @@ -472,11 +473,9 @@ fn emit_padding(bit_size: &TokenStream) -> TokenStream { )) )?; - if __deku_rest[__deku_total_read..].len() >= __deku_pad { - __deku_total_read += __deku_pad; - } else { - return Err(::#crate_::DekuError::Incomplete(::#crate_::error::NeedSize::new(__deku_pad))); - } + + // TODO: This could be bytes + container.read_bits(__deku_pad)?; } } } @@ -657,7 +656,7 @@ fn emit_field_read( let field_read_normal = quote! { let __deku_value = #field_read_func?; - //let __deku_value: #field_type = #field_map(__deku_value)?; + let __deku_value: #field_type = #field_map(__deku_value)?; __deku_value }; diff --git a/examples/custom_reader_and_writer.rs b/examples/custom_reader_and_writer.rs index 0bb8dbd4..d423e404 100644 --- a/examples/custom_reader_and_writer.rs +++ b/examples/custom_reader_and_writer.rs @@ -1,25 +1,22 @@ use std::convert::TryInto; -use deku::bitvec::{BitSlice, BitVec, Msb0}; +use deku::bitvec::{BitVec, Msb0}; use deku::ctx::BitSize; use deku::prelude::*; -fn bit_flipper_read( +fn bit_flipper_read( field_a: u8, - rest: &BitSlice, + container: &mut Container, bit_size: BitSize, ) -> Result { // Access to previously read fields println!("field_a = 0x{:X}", field_a); - // The current rest - println!("rest = {:?}", rest); - // Size of the current field println!("bit_size: {:?}", bit_size); // read field_b, calling original func - let (amt_read, value) = u8::read(rest, bit_size)?; + let value = u8::from_reader(container, bit_size)?; // flip the bits on value if field_a is 0x01 let value = if field_a == 0x01 { !value } else { value }; @@ -53,7 +50,7 @@ struct DekuTest { field_a: u8, #[deku( - reader = "bit_flipper_read(*field_a, deku::rest, BitSize(8))", + reader = "bit_flipper_read(*field_a, deku::reader, BitSize(8))", writer = "bit_flipper_write(*field_a, *field_b, deku::output, BitSize(8))" )] field_b: u8, diff --git a/examples/example.rs b/examples/example.rs index 526e9211..58fe725a 100644 --- a/examples/example.rs +++ b/examples/example.rs @@ -11,6 +11,7 @@ use deku::{ #[derive(Debug, PartialEq, DekuRead, DekuWrite)] struct FieldF { #[deku(bits = "6")] + #[deku(assert_eq = "6")] data: u8, } diff --git a/src/container.rs b/src/container.rs index 7f84af2a..07935411 100644 --- a/src/container.rs +++ b/src/container.rs @@ -1,5 +1,7 @@ use bitvec::prelude::*; -use std::io::Read; +use std::io::{self, Read}; + +use crate::{prelude::NeedSize, DekuError}; pub enum ContainerRet { Bytes, @@ -23,7 +25,11 @@ impl Container { } } - pub fn read_bits(&mut self, amt: usize) -> std::io::Result> { + #[inline] + pub fn read_bits(&mut self, amt: usize) -> Result, DekuError> { + if amt == 0 { + return Ok(BitVec::new()); + } let mut ret = BitVec::with_capacity(amt); if amt < self.leftover.len() { @@ -34,12 +40,16 @@ impl Container { ret.extend(self.leftover.clone()); let bits_left = amt - self.leftover.len(); - let mut bytes_len = (bits_left / 8); + let mut bytes_len = bits_left / 8; if (bits_left % 8) != 0 { bytes_len += 1; } let mut buf = vec![0; bytes_len]; - self.inner.read_exact(&mut buf); + if let Err(e) = self.inner.read_exact(&mut buf) { + if e.kind() == io::ErrorKind::UnexpectedEof { + return Err(DekuError::Incomplete(NeedSize::new(amt))); + } + } let mut rest: BitVec = BitVec::try_from_slice(&buf).unwrap(); let add = rest.split_off(bits_left); @@ -56,30 +66,19 @@ impl Container { // // 1. We must have no leftover bits, so that we are "aligned" #[inline] - pub fn read_bytes(&mut self, amt: usize, buf: &mut [u8]) -> ContainerRet { + pub fn read_bytes(&mut self, amt: usize, buf: &mut [u8]) -> Result { if self.leftover.is_empty() { - self.inner.read_exact(buf); - ContainerRet::Bytes + if let Err(e) = self.inner.read_exact(&mut buf[..amt]) { + if e.kind() == io::ErrorKind::UnexpectedEof { + return Err(DekuError::Incomplete(NeedSize::new(amt * 8))); + } + + // TODO: other errors? + } + self.bits_read += amt * 8; + Ok(ContainerRet::Bytes) } else { - ContainerRet::Bits(self.read_bits(amt * 8).unwrap()) + Ok(ContainerRet::Bits(self.read_bits(amt * 8)?)) } } } - -#[cfg(test)] -mod tests { - use super::*; - #[test] - fn test_container() { - use std::io::Cursor; - let buf = [0x12, 0x34]; - let buf = std::io::Cursor::new(buf); - - let mut container = Container::new(buf); - - let bits = container.read_bits(4).unwrap(); - let bits = container.read_bits(4).unwrap(); - let bits = container.read_bits(4).unwrap(); - let bits = container.read_bits(4).unwrap(); - } -} diff --git a/src/error.rs b/src/error.rs index 061f58dc..63143bed 100644 --- a/src/error.rs +++ b/src/error.rs @@ -6,6 +6,8 @@ use alloc::format; use alloc::string::String; /// Number of bits needed to retry parsing +/// +/// TODO: add bytes #[derive(Debug, Clone, PartialEq, Eq)] pub struct NeedSize { bits: usize, diff --git a/src/impls/primitive.rs b/src/impls/primitive.rs index 6e42b849..42ed671a 100644 --- a/src/impls/primitive.rs +++ b/src/impls/primitive.rs @@ -34,9 +34,9 @@ impl DekuRead<'_, (Endian, ByteSize)> for u8 { (endian, size): (Endian, ByteSize), ) -> Result { let mut buf = [0; core::mem::size_of::()]; - let ret = container.read_bytes(size.0, &mut buf); + let ret = container.read_bytes(size.0, &mut buf)?; let a = match ret { - ContainerRet::Bits(mut bits) => { + ContainerRet::Bits(bits) => { let a = ::read(&bits, (endian, size))?; a.1 } @@ -139,7 +139,7 @@ macro_rules! ImplDekuReadBits { container: &mut crate::container::Container, (endian, size): (Endian, BitSize), ) -> Result<$typ, DekuError> { - let mut bits = container.read_bits(size.0).unwrap(); + let bits = container.read_bits(size.0)?; let a = <$typ>::read(&bits, (endian, size))?; Ok(a.1) } @@ -222,14 +222,18 @@ macro_rules! ImplDekuReadBytes { (endian, size): (Endian, ByteSize), ) -> Result<$typ, DekuError> { let mut buf = [0; core::mem::size_of::<$typ>()]; - let ret = container.read_bytes(size.0, &mut buf); + let ret = container.read_bytes(size.0, &mut buf)?; let a = match ret { - ContainerRet::Bits(mut bits) => { + ContainerRet::Bits(bits) => { let a = <$typ>::read(&bits, (endian, size))?; a.1 } ContainerRet::Bytes => { - <$typ>::from_be_bytes(buf.try_into().unwrap()) + if endian.is_le() { + <$typ>::from_le_bytes(buf.try_into().unwrap()) + } else { + <$typ>::from_be_bytes(buf.try_into().unwrap()) + } } }; Ok(a) @@ -262,7 +266,7 @@ macro_rules! ImplDekuReadSignExtend { (endian, size): (Endian, ByteSize), ) -> Result<$typ, DekuError> { // TODO: specialize for reading byte aligned - let mut bits = container.read_bits(size.0 * 8).unwrap(); + let bits = container.read_bits(size.0 * 8)?; let a = <$typ>::read(&bits, (endian, size))?; Ok(a.1) } @@ -289,7 +293,7 @@ macro_rules! ImplDekuReadSignExtend { container: &mut crate::container::Container, (endian, size): (Endian, BitSize), ) -> Result<$typ, DekuError> { - let mut bits = container.read_bits(size.0 * 8).unwrap(); + let bits = container.read_bits(size.0 * 8)?; let a = <$typ>::read(&bits, (endian, size))?; Ok(a.1) } diff --git a/src/impls/vec.rs b/src/impls/vec.rs index c1914764..fe5289ce 100644 --- a/src/impls/vec.rs +++ b/src/impls/vec.rs @@ -68,7 +68,7 @@ where { let mut res = capacity.map_or_else(Vec::new, Vec::with_capacity); - let mut start_read = container.bits_read; + let start_read = container.bits_read; loop { let val = ::from_reader(container, ctx)?; diff --git a/src/prelude.rs b/src/prelude.rs index 3ac88025..11d6f6c6 100644 --- a/src/prelude.rs +++ b/src/prelude.rs @@ -4,6 +4,6 @@ */ pub use crate::error::{DekuError, NeedSize}; pub use crate::{ - deku_derive, DekuContainerRead, DekuContainerWrite, DekuEnumExt, DekuRead, DekuUpdate, - DekuWrite, + container::Container, deku_derive, DekuContainerRead, DekuContainerWrite, DekuEnumExt, + DekuRead, DekuUpdate, DekuWrite, }; diff --git a/tests/test_attributes/test_ctx.rs b/tests/test_attributes/test_ctx.rs index 43114f26..213f8b04 100644 --- a/tests/test_attributes/test_ctx.rs +++ b/tests/test_attributes/test_ctx.rs @@ -1,7 +1,9 @@ use std::convert::{TryFrom, TryInto}; +use std::io::Cursor; use bitvec::bitvec; use deku::bitvec::{BitView, Msb0}; +use deku::container::Container; use deku::prelude::*; /// General smoke tests for ctx @@ -13,7 +15,7 @@ fn test_ctx_struct() { #[deku(ctx = "a: u8, b: u8")] struct SubTypeNeedCtx { #[deku( - reader = "(|rest|{u8::read(rest,()).map(|(slice,c)|(slice,(a+b+c) as usize))})(deku::rest)", + reader = "(u8::from_reader(container,()).map(|c|(a+b+c) as usize))", writer = "(|c|{u8::write(&(c-a-b), deku::output, ())})(self.i as u8)" )] i: usize, @@ -53,7 +55,7 @@ fn test_top_level_ctx_enum() { #[deku(id = "1")] VariantA( #[deku( - reader = "(|rest|{u8::read(rest,()).map(|(slice,c)|(slice,(a+b+c)))})", + reader = "(u8::from_reader(container,()).map(|c|(a+b+c)))", writer = "(|c|{u8::write(&(c-a-b), deku::output, ())})(field_0)" )] u8, @@ -61,9 +63,8 @@ fn test_top_level_ctx_enum() { } let test_data = [0x01_u8, 0x03]; - let bit_slice = test_data.view_bits(); - let (amt_read, ret_read) = TopLevelCtxEnum::read(bit_slice, (1, 2)).unwrap(); - assert!(bit_slice[amt_read..].is_empty()); + let ret_read = + TopLevelCtxEnum::from_reader(&mut Container::new(Cursor::new(test_data)), (1, 2)).unwrap(); assert_eq!(ret_read, TopLevelCtxEnum::VariantA(0x06)); let mut ret_write = bitvec![u8, Msb0;]; @@ -79,7 +80,7 @@ fn test_top_level_ctx_enum_default() { #[deku(id = "1")] VariantA( #[deku( - reader = "(|rest|{u8::read(rest,()).map(|(slice,c)|(slice,(a+b+c)))})", + reader = "(u8::from_reader(container, ()).map(|c|(a+b+c)))", writer = "(|c|{u8::write(&(c-a-b), deku::output, ())})(field_0)" )] u8, @@ -96,9 +97,9 @@ fn test_top_level_ctx_enum_default() { assert_eq!(test_data.to_vec(), ret_write); // Use context - let bit_slice = test_data.view_bits(); - let (amt_read, ret_read) = TopLevelCtxEnumDefault::read(bit_slice, (1, 2)).unwrap(); - assert!(bit_slice[amt_read..].is_empty()); + let ret_read = + TopLevelCtxEnumDefault::from_reader(&mut Container::new(Cursor::new(test_data)), (1, 2)) + .unwrap(); assert_eq!(ret_read, TopLevelCtxEnumDefault::VariantA(0x06)); let mut ret_write = bitvec![u8, Msb0;]; ret_read.write(&mut ret_write, (1, 2)).unwrap(); @@ -215,9 +216,9 @@ fn test_ctx_default_struct() { assert_eq!(ret_write, test_data); // Use context - let bit_slice = test_data.view_bits(); - let (amt_read, ret_read) = TopLevelCtxStructDefault::read(bit_slice, (1, 2)).unwrap(); - assert_eq!(bit_slice.len(), amt_read); + let ret_read = + TopLevelCtxStructDefault::from_reader(&mut Container::new(Cursor::new(test_data)), (1, 2)) + .unwrap(); assert_eq!(expected, ret_read); let mut ret_write = bitvec![u8, Msb0;]; ret_read.write(&mut ret_write, (1, 2)).unwrap(); diff --git a/tests/test_compile/cases/internal_variables.rs b/tests/test_compile/cases/internal_variables.rs index 4e5d1930..c9aa76aa 100644 --- a/tests/test_compile/cases/internal_variables.rs +++ b/tests/test_compile/cases/internal_variables.rs @@ -1,5 +1,5 @@ use deku::prelude::*; -use deku::bitvec::{BitVec, BitSlice, Msb0}; +use deku::bitvec::{BitVec, Msb0}; #[derive(DekuRead, DekuWrite)] struct TestCount { @@ -50,16 +50,16 @@ struct TestMap { field_b: usize } -fn dummy_reader( +fn dummy_reader( offset: usize, - rest: &BitSlice, -) -> Result<(usize, usize), DekuError> { - Ok((0, offset)) + _reader: &mut Container, +) -> Result { + Ok(0) } #[derive(DekuRead, DekuWrite)] struct TestReader { field_a: u8, - #[deku(reader = "dummy_reader(deku::byte_offset, deku::rest)")] + #[deku(reader = "dummy_reader(deku::byte_offset, deku::reader)")] field_b: usize }