From 749f7521cdb2c2769483e8135eefcc298e0da262 Mon Sep 17 00:00:00 2001 From: Will Glynn Date: Thu, 1 Nov 2018 18:22:16 -0500 Subject: [PATCH 1/4] Borrow instead of copying in IntoCtx, TryIntoCtx, Cwrite, Pwrite --- scroll_derive/examples/main.rs | 2 +- scroll_derive/src/lib.rs | 27 +++-------- scroll_derive/tests/tests.rs | 4 +- src/ctx.rs | 82 +++++++++++++++------------------- src/greater.rs | 22 ++++----- src/lib.rs | 16 +++---- src/pwrite.rs | 16 +++---- tests/api.rs | 16 +++---- 8 files changed, 81 insertions(+), 104 deletions(-) diff --git a/scroll_derive/examples/main.rs b/scroll_derive/examples/main.rs index e0955d5..48b81ad 100644 --- a/scroll_derive/examples/main.rs +++ b/scroll_derive/examples/main.rs @@ -18,7 +18,7 @@ fn main () { println!("data: {:?}", &data); assert_eq!(data.id, 0xdeadbeefu32); let mut bytes2 = vec![0; ::std::mem::size_of::()]; - bytes2.pwrite_with(data, 0, LE).unwrap(); + bytes2.pwrite_with(&data, 0, LE).unwrap(); let data: Data = bytes.pread_with(0, LE).unwrap(); let data2: Data = bytes2.pread_with(0, LE).unwrap(); assert_eq!(data, data2); diff --git a/scroll_derive/src/lib.rs b/scroll_derive/src/lib.rs index f2c3477..d3ac09b 100644 --- a/scroll_derive/src/lib.rs +++ b/scroll_derive/src/lib.rs @@ -91,24 +91,16 @@ fn impl_try_into_ctx(name: &syn::Ident, fields: &syn::FieldsNamed) -> proc_macro }).collect(); quote! { - impl<'a> ::scroll::ctx::TryIntoCtx<::scroll::Endian> for &'a #name { + impl ::scroll::ctx::TryIntoCtx<::scroll::Endian> for #name { type Error = ::scroll::Error; #[inline] - fn try_into_ctx(self, dst: &mut [u8], ctx: ::scroll::Endian) -> ::scroll::export::result::Result { + fn try_into_ctx(&self, dst: &mut [u8], ctx: ::scroll::Endian) -> ::scroll::export::result::Result { use ::scroll::Pwrite; let offset = &mut 0; #(#items;)*; Ok(*offset) } } - - impl ::scroll::ctx::TryIntoCtx<::scroll::Endian> for #name { - type Error = ::scroll::Error; - #[inline] - fn try_into_ctx(self, dst: &mut [u8], ctx: ::scroll::Endian) -> ::scroll::export::result::Result { - (&self).try_into_ctx(dst, ctx) - } - } } } @@ -275,14 +267,14 @@ fn impl_into_ctx(name: &syn::Ident, fields: &syn::FieldsNamed) -> proc_macro2::T quote! { let size = ::scroll::export::mem::size_of::<#arrty>(); for i in 0..self.#ident.len() { - dst.cwrite_with(self.#ident[i], *offset, ctx); + dst.cwrite_with(&self.#ident[i], *offset, ctx); *offset += size; } } }, _ => { quote! { - dst.cwrite_with(self.#ident, *offset, ctx); + dst.cwrite_with(&self.#ident, *offset, ctx); *offset += #size; } } @@ -290,22 +282,15 @@ fn impl_into_ctx(name: &syn::Ident, fields: &syn::FieldsNamed) -> proc_macro2::T }).collect(); quote! { - impl<'a> ::scroll::ctx::IntoCtx<::scroll::Endian> for &'a #name { + impl ::scroll::ctx::IntoCtx<::scroll::Endian> for #name { #[inline] - fn into_ctx(self, dst: &mut [u8], ctx: ::scroll::Endian) { + fn into_ctx(&self, dst: &mut [u8], ctx: ::scroll::Endian) { use ::scroll::Cwrite; let offset = &mut 0; #(#items;)*; () } } - - impl ::scroll::ctx::IntoCtx<::scroll::Endian> for #name { - #[inline] - fn into_ctx(self, dst: &mut [u8], ctx: ::scroll::Endian) { - (&self).into_ctx(dst, ctx) - } - } } } diff --git a/scroll_derive/tests/tests.rs b/scroll_derive/tests/tests.rs index d54f77e..cd22078 100644 --- a/scroll_derive/tests/tests.rs +++ b/scroll_derive/tests/tests.rs @@ -19,7 +19,7 @@ fn test_data (){ assert_eq!(data.id, 0xdeadbeefu32); assert_eq!(data.timestamp, 0.5f64); let mut bytes2 = vec![0; ::std::mem::size_of::()]; - bytes2.pwrite_with(data, 0, LE).unwrap(); + bytes2.pwrite_with(&data, 0, LE).unwrap(); let data: Data = bytes.pread_with(0, LE).unwrap(); let data2: Data = bytes2.pread_with(0, LE).unwrap(); assert_eq!(data, data2); @@ -83,7 +83,7 @@ fn test_iowrite (){ assert_eq!(bytes_null, bytes); let mut bytes_null = [0u8; 8]; - bytes_null.cwrite_with(data, 0, LE); + bytes_null.cwrite_with(&data, 0, LE); println!("bytes_null: {:?}", &bytes_null); println!("bytes : {:?}", &bytes); assert_eq!(bytes_null, bytes); diff --git a/src/ctx.rs b/src/ctx.rs index e370c79..67e45de 100644 --- a/src/ctx.rs +++ b/src/ctx.rs @@ -127,14 +127,14 @@ pub trait TryFromCtx<'a, Ctx: Copy = (), This: ?Sized = [u8]> where Self: 'a + S } /// Writes `Self` into `This` using the context `Ctx` -pub trait IntoCtx: Sized { - fn into_ctx(self, &mut This, ctx: Ctx); +pub trait IntoCtx { + fn into_ctx(&self, &mut This, ctx: Ctx); } /// Tries to write `Self` into `This` using the context `Ctx` -pub trait TryIntoCtx: Sized { +pub trait TryIntoCtx { type Error; - fn try_into_ctx(self, &mut This, ctx: Ctx) -> Result; + fn try_into_ctx(&self, &mut This, ctx: Ctx) -> Result; } /// Gets the size of `Self` with a `Ctx`, and in `Self::Units`. Implementors can then call `Gread` related functions @@ -176,21 +176,15 @@ macro_rules! into_ctx_impl { ($typ:tt, $size:expr) => { impl IntoCtx for $typ { #[inline] - fn into_ctx(self, dst: &mut [u8], le: Endian) { + fn into_ctx(&self, dst: &mut [u8], le: Endian) { assert!(dst.len() >= $size); write_into!($typ, $size, self, dst, le); } } - impl<'a> IntoCtx for &'a $typ { - #[inline] - fn into_ctx(self, dst: &mut [u8], le: Endian) { - (*self).into_ctx(dst, le) - } - } impl TryIntoCtx for $typ where $typ: IntoCtx { type Error = error::Error; #[inline] - fn try_into_ctx(self, dst: &mut [u8], le: Endian) -> error::Result { + fn try_into_ctx(&self, dst: &mut [u8], le: Endian) -> error::Result { if $size > dst.len () { Err(error::Error::TooBig{size: $size, len: dst.len()}) } else { @@ -199,13 +193,6 @@ macro_rules! into_ctx_impl { } } } - impl<'a> TryIntoCtx for &'a $typ { - type Error = error::Error; - #[inline] - fn try_into_ctx(self, dst: &mut [u8], le: Endian) -> error::Result { - (*self).try_into_ctx(dst, le) - } - } } } @@ -334,21 +321,15 @@ macro_rules! into_ctx_float_impl { ($typ:tt, $size:expr) => { impl IntoCtx for $typ { #[inline] - fn into_ctx(self, dst: &mut [u8], le: Endian) { + fn into_ctx(&self, dst: &mut [u8], le: Endian) { assert!(dst.len() >= $size); - write_into!(signed_to_unsigned!($typ), $size, transmute::<$typ, signed_to_unsigned!($typ)>(self), dst, le); - } - } - impl<'a> IntoCtx for &'a $typ { - #[inline] - fn into_ctx(self, dst: &mut [u8], le: Endian) { - (*self).into_ctx(dst, le) + write_into!(signed_to_unsigned!($typ), $size, transmute::<$typ, signed_to_unsigned!($typ)>(*self), dst, le); } } impl TryIntoCtx for $typ where $typ: IntoCtx { type Error = error::Error; #[inline] - fn try_into_ctx(self, dst: &mut [u8], le: Endian) -> error::Result { + fn try_into_ctx(&self, dst: &mut [u8], le: Endian) -> error::Result { if $size > dst.len () { Err(error::Error::TooBig{size: $size, len: dst.len()}) } else { @@ -357,13 +338,6 @@ macro_rules! into_ctx_float_impl { } } } - impl<'a> TryIntoCtx for &'a $typ { - type Error = error::Error; - #[inline] - fn try_into_ctx(self, dst: &mut [u8], le: Endian) -> error::Result { - (*self).try_into_ctx(dst, le) - } - } } } @@ -410,10 +384,10 @@ impl<'a, T> TryFromCtx<'a, StrCtx, T> for &'a str where T: AsRef<[u8]> { } } -impl<'a> TryIntoCtx for &'a [u8] { +impl TryIntoCtx for [u8] { type Error = error::Error; #[inline] - fn try_into_ctx(self, dst: &mut [u8], _ctx: ()) -> error::Result { + fn try_into_ctx(&self, dst: &mut [u8], _ctx: ()) -> error::Result { let src_len = self.len() as isize; let dst_len = dst.len() as isize; // if src_len < 0 || dst_len < 0 || offset < 0 { @@ -429,15 +403,33 @@ impl<'a> TryIntoCtx for &'a [u8] { } // TODO: make TryIntoCtx use StrCtx for awesomeness -impl<'a> TryIntoCtx for &'a str { +impl TryIntoCtx for str { type Error = error::Error; #[inline] - fn try_into_ctx(self, dst: &mut [u8], _ctx: ()) -> error::Result { + fn try_into_ctx(&self, dst: &mut [u8], _ctx: ()) -> error::Result { let bytes = self.as_bytes(); - TryIntoCtx::try_into_ctx(bytes, dst, ()) + TryIntoCtx::try_into_ctx(&bytes, dst, ()) + } +} + +// Implement borrowed versions for these types for convenience +impl<'a> TryIntoCtx for &'a [u8] { + type Error = error::Error; + #[inline] + fn try_into_ctx(&self, dst: &mut [u8], ctx: ()) -> error::Result { + (*self).try_into_ctx(dst, ctx) } } +impl<'a> TryIntoCtx for &'a str { + type Error = error::Error; + #[inline] + fn try_into_ctx(&self, dst: &mut [u8], ctx: ()) -> error::Result { + (*self).try_into_ctx(dst, ctx) + } +} + + // TODO: we can make this compile time without size_of call, but compiler probably does that anyway macro_rules! sizeof_impl { ($ty:ty) => { @@ -510,7 +502,7 @@ impl<'a> TryFromCtx<'a, usize> for &'a[u8] { impl IntoCtx for usize { #[inline] - fn into_ctx(self, dst: &mut [u8], le: Endian) { + fn into_ctx(&self, dst: &mut [u8], le: Endian) { let size = ::core::mem::size_of::(); assert!(dst.len() >= size); let mut data = if le.is_little() { self.to_le() } else { self.to_be() }; @@ -524,7 +516,7 @@ impl IntoCtx for usize { impl TryIntoCtx for usize where usize: IntoCtx { type Error = error::Error; #[inline] - fn try_into_ctx(self, dst: &mut [u8], le: Endian) -> error::Result { + fn try_into_ctx(&self, dst: &mut [u8], le: Endian) -> error::Result { let size = ::core::mem::size_of::(); if size > dst.len() { Err(error::Error::TooBig{size, len: dst.len()}) @@ -564,10 +556,10 @@ impl<'a> TryFromCtx<'a> for CString { } #[cfg(feature = "std")] -impl<'a> TryIntoCtx for &'a CStr { +impl TryIntoCtx for CStr { type Error = error::Error; #[inline] - fn try_into_ctx(self, dst: &mut [u8], _ctx: ()) -> error::Result { + fn try_into_ctx(&self, dst: &mut [u8], _ctx: ()) -> error::Result { let data = self.to_bytes_with_nul(); if dst.len() < data.len() { @@ -589,7 +581,7 @@ impl<'a> TryIntoCtx for &'a CStr { impl TryIntoCtx for CString { type Error = error::Error; #[inline] - fn try_into_ctx(self, dst: &mut [u8], _ctx: ()) -> error::Result { + fn try_into_ctx(&self, dst: &mut [u8], _ctx: ()) -> error::Result { self.as_c_str().try_into_ctx(dst, ()) } } diff --git a/src/greater.rs b/src/greater.rs index c0f231e..a805ad3 100644 --- a/src/greater.rs +++ b/src/greater.rs @@ -101,16 +101,16 @@ impl + Index>> Cread for /// } /// /// impl ctx::IntoCtx for Bar { -/// fn into_ctx(self, bytes: &mut [u8], ctx: scroll::Endian) { +/// fn into_ctx(&self, bytes: &mut [u8], ctx: scroll::Endian) { /// use scroll::Cwrite; -/// bytes.cwrite_with(self.foo, 0, ctx); -/// bytes.cwrite_with(self.bar, 4, ctx); +/// bytes.cwrite_with(&self.foo, 0, ctx); +/// bytes.cwrite_with(&self.bar, 4, ctx); /// } /// } /// /// let bar = Bar { foo: -1, bar: 0xdeadbeef }; /// let mut bytes = [0x0; 16]; -/// bytes.cwrite::(bar, 0); +/// bytes.cwrite::(&bar, 0); /// ``` pub trait Cwrite: Index + IndexMut> { /// Writes `n` into `Self` at `offset`; uses default context. @@ -121,15 +121,15 @@ pub trait Cwrite: Index + IndexMut> { /// ``` /// use scroll::{Cwrite, Cread}; /// let mut bytes = [0x0; 16]; - /// bytes.cwrite::(42, 0); - /// bytes.cwrite::(0xdeadbeef, 8); + /// bytes.cwrite::(&42, 0); + /// bytes.cwrite::(&0xdeadbeef, 8); /// /// assert_eq!(bytes.cread::(0), 42); /// assert_eq!(bytes.cread::(8), 0xdeadbeef); #[inline] - fn cwrite>>::Output>>(&mut self, n: N, offset: I) where Ctx: Default { + fn cwrite>>::Output>>(&mut self, n: &N, offset: I) where Ctx: Default { let ctx = Ctx::default(); - n.into_ctx(self.index_mut(offset..), ctx) + n.into_ctx(self.index_mut( offset..), ctx) } /// Writes `n` into `Self` at `offset` with `ctx` /// @@ -138,12 +138,12 @@ pub trait Cwrite: Index + IndexMut> { /// ``` /// use scroll::{Cwrite, Cread, LE, BE}; /// let mut bytes = [0x0; 0x10]; - /// bytes.cwrite_with::(42, 0, LE); - /// bytes.cwrite_with::(0xdeadbeef, 8, BE); + /// bytes.cwrite_with::(&42, 0, LE); + /// bytes.cwrite_with::(&0xdeadbeef, 8, BE); /// assert_eq!(bytes.cread_with::(0, LE), 42); /// assert_eq!(bytes.cread_with::(8, LE), 0xefbeadde); #[inline] - fn cwrite_with>>::Output>>(&mut self, n: N, offset: I, ctx: Ctx) { + fn cwrite_with>>::Output>>(&mut self, n: &N, offset: I, ctx: Ctx) { n.into_ctx(self.index_mut(offset..), ctx) } } diff --git a/src/lib.rs b/src/lib.rs index d01c222..16b2561 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -203,9 +203,9 @@ mod tests { use super::{Pwrite, Pread, BE}; let mut bytes: [u8; 8] = [0, 0, 0, 0, 0, 0, 0, 0]; let b = &mut bytes[..]; - b.pwrite_with::<$read>($deadbeef, 0, LE).unwrap(); + b.pwrite_with::<$read>(&$deadbeef, 0, LE).unwrap(); assert_eq!(b.pread_with::<$read>(0, LE).unwrap(), $deadbeef); - b.pwrite_with::<$read>($deadbeef, 0, BE).unwrap(); + b.pwrite_with::<$read>(&$deadbeef, 0, BE).unwrap(); assert_eq!(b.pread_with::<$read>(0, BE).unwrap(), $deadbeef); } } @@ -303,13 +303,13 @@ mod tests { use super::ctx::*; let astring: &str = "lol hello_world lal\0ala imabytes"; let mut buffer = [0u8; 33]; - buffer.pwrite(astring, 0).unwrap(); + buffer.pwrite(&astring, 0).unwrap(); { let hello_world = buffer.pread_with::<&str>(4, StrCtx::Delimiter(SPACE)).unwrap(); assert_eq!(hello_world, "hello_world"); } let bytes: &[u8] = b"more\0bytes"; - buffer.pwrite(bytes, 0).unwrap(); + buffer.pwrite(&bytes, 0).unwrap(); let more = bytes.pread_with::<&str>(0, StrCtx::Delimiter(NULL)).unwrap(); assert_eq!(more, "more"); let bytes = bytes.pread_with::<&str>(more.len() + 1, StrCtx::Delimiter(NULL)).unwrap(); @@ -349,10 +349,10 @@ mod tests { impl super::ctx::TryIntoCtx for Foo { type Error = ExternalError; - fn try_into_ctx(self, this: &mut [u8], le: super::Endian) -> Result { + fn try_into_ctx(&self, this: &mut [u8], le: super::Endian) -> Result { use super::Pwrite; if this.len() < 2 { return Err((ExternalError {}).into()) } - this.pwrite_with(self.0, 0, le)?; + this.pwrite_with(&self.0, 0, le)?; Ok(2) } } @@ -430,14 +430,14 @@ mod tests { use super::{LE, BE, Pread, Pwrite}; let mut buffer = [0u8; 16]; let offset = &mut 0; - buffer.gwrite_with($val.clone(), offset, LE).unwrap(); + buffer.gwrite_with(&$val, offset, LE).unwrap(); 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!(*o2, *offset); - buffer.gwrite_with($val.clone(), offset, BE).unwrap(); + buffer.gwrite_with(&$val, offset, BE).unwrap(); let val: $typ = buffer.gread_with(o2, BE).unwrap(); assert_eq!(val, $val); } diff --git a/src/pwrite.rs b/src/pwrite.rs index 51e95b0..96243b1 100644 --- a/src/pwrite.rs +++ b/src/pwrite.rs @@ -18,23 +18,23 @@ use error; /// // you can use your own error here too, but you will then need to specify it in fn generic parameters /// type Error = scroll::Error; /// // you can write using your own context too... see `leb128.rs` -/// fn try_into_ctx(self, this: &mut [u8], le: Endian) -> Result { +/// fn try_into_ctx(&self, this: &mut [u8], le: Endian) -> Result { /// if this.len() < 2 { return Err((scroll::Error::Custom("whatever".to_string())).into()) } -/// this.pwrite_with(self.0, 0, le)?; +/// this.pwrite_with(&self.0, 0, le)?; /// Ok(2) /// } /// } /// // now we can write a `Foo` into some buffer (in this case, a byte buffer, because that's what we implemented it for above) /// /// let mut bytes: [u8; 4] = [0, 0, 0, 0]; -/// bytes.pwrite_with(Foo(0x7f), 1, LE).unwrap(); +/// bytes.pwrite_with(&Foo(0x7f), 1, LE).unwrap(); /// pub trait Pwrite : Index + IndexMut> + MeasureWith where Ctx: Copy, E: From, { - fn pwrite>>::Output, Error = E>>(&mut self, n: N, offset: usize) -> result::Result where Ctx: Default { + fn pwrite>>::Output, Error = E>>(&mut self, n: &N, offset: usize) -> result::Result where Ctx: Default { self.pwrite_with(n, offset, Ctx::default()) } /// Write `N` at offset `I` with context `Ctx` @@ -42,9 +42,9 @@ pub trait Pwrite : Index + IndexMut> + MeasureWi /// ``` /// use scroll::{Pwrite, Pread, LE}; /// let mut bytes: [u8; 8] = [0, 0, 0, 0, 0, 0, 0, 0]; - /// bytes.pwrite_with::(0xbeefbeef, 0, LE).unwrap(); + /// bytes.pwrite_with::(&0xbeefbeef, 0, LE).unwrap(); /// assert_eq!(bytes.pread_with::(0, LE).unwrap(), 0xbeefbeef); - fn pwrite_with>>::Output, Error = E>>(&mut self, n: N, offset: usize, ctx: Ctx) -> result::Result { + fn pwrite_with>>::Output, Error = E>>(&mut self, n: &N, offset: usize, ctx: Ctx) -> result::Result { let len = self.measure_with(&ctx); if offset >= len { return Err(error::Error::BadOffset(offset).into()) @@ -54,14 +54,14 @@ pub trait Pwrite : Index + IndexMut> + MeasureWi } /// Write `n` into `self` at `offset`, with a default `Ctx`. Updates the offset. #[inline] - fn gwrite>>::Output, Error = E>>(&mut self, n: N, offset: &mut usize) -> result::Result where + fn gwrite>>::Output, Error = E>>(&mut self, n: &N, offset: &mut usize) -> result::Result where Ctx: Default { let ctx = Ctx::default(); self.gwrite_with(n, offset, ctx) } /// Write `n` into `self` at `offset`, with the `ctx`. Updates the offset. #[inline] - fn gwrite_with>>::Output, Error = E>>(&mut self, n: N, offset: &mut usize, ctx: Ctx) -> result::Result { + fn gwrite_with>>::Output, Error = E>>(&mut self, n: &N, offset: &mut usize, ctx: Ctx) -> result::Result { let o = *offset; match self.pwrite_with(n, o, ctx) { Ok(size) => { diff --git a/tests/api.rs b/tests/api.rs index 7c79dbc..19fe7df 100644 --- a/tests/api.rs +++ b/tests/api.rs @@ -206,7 +206,6 @@ fn ioread_api() { assert_eq!({foo_.bar}, bar); } -#[repr(packed)] struct Bar { foo: i32, bar: u32, @@ -251,17 +250,18 @@ fn cwrite_api() { use scroll::Cwrite; use scroll::Cread; let mut bytes = [0x0; 16]; - bytes.cwrite::(42, 0); - bytes.cwrite::(0xdeadbeef, 8); + bytes.cwrite::(&42, 0); + bytes.cwrite::(&0xdeadbeef, 8); assert_eq!(bytes.cread::(0), 42); assert_eq!(bytes.cread::(8), 0xdeadbeef); } impl scroll::ctx::IntoCtx for Bar { - fn into_ctx(self, bytes: &mut [u8], ctx: scroll::Endian) { + fn into_ctx(&self, bytes: &mut [u8], ctx: scroll::Endian) { use scroll::Cwrite; - bytes.cwrite_with(self.foo, 0, ctx); - bytes.cwrite_with(self.bar, 4, ctx); + let Self{foo, bar} = *self; + bytes.cwrite_with(&foo, 0, ctx); + bytes.cwrite_with(&bar, 4, ctx); } } @@ -269,8 +269,8 @@ impl scroll::ctx::IntoCtx for Bar { fn cwrite_api_customtype() { use scroll::{Cwrite, Cread}; let bar = Bar { foo: -1, bar: 0xdeadbeef }; - let mut bytes = [0x0; 16]; - &bytes[..].cwrite::(bar, 0); + let mut bytes = [0u8; 16]; + &bytes[..].cwrite::(&bar, 0); let bar = bytes.cread::(0); assert_eq!({bar.foo}, -1); assert_eq!({bar.bar}, 0xdeadbeef); From ad6df6e7272978208be0749ef0c3ad244ac9bffe Mon Sep 17 00:00:00 2001 From: Will Glynn Date: Thu, 1 Nov 2018 19:46:12 -0500 Subject: [PATCH 2/4] Add a test which complains about unaligned field accesses --- scroll_derive/tests/tests.rs | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/scroll_derive/tests/tests.rs b/scroll_derive/tests/tests.rs index cd22078..31e158b 100644 --- a/scroll_derive/tests/tests.rs +++ b/scroll_derive/tests/tests.rs @@ -158,3 +158,32 @@ fn test_nested_struct() { assert_eq!(read, size); assert_eq!(b, b2); } + +#[derive(Debug,Copy,Clone,Pwrite,Pread,IOwrite,IOread)] +#[repr(packed)] +struct PackedStruct { + a: u8, + b: u32, +} + +#[test] +fn cwrite_packed_struct() { + use scroll::{Cwrite, Cread}; + let mut bytes = [0u8; 5]; + &bytes[..].cwrite(&PackedStruct{ a: 1, b: 2 }, 0); + + let PackedStruct{ a, b } = bytes.cread(0); + assert_eq!(a, 1); + assert_eq!(b, 2); +} + +#[test] +fn pwrite_packed_struct() { + use scroll::{Pwrite, Pread}; + let mut bytes = [0u8; 5]; + &bytes[..].pwrite(&PackedStruct{ a: 1, b: 2 }, 0).unwrap(); + + let PackedStruct{ a, b } = bytes.pread(0).unwrap(); + assert_eq!(a, 1); + assert_eq!(b, 2); +} From 2a73c267be14975a0dcabc45c7daf8f1c83e1d63 Mon Sep 17 00:00:00 2001 From: Will Glynn Date: Thu, 1 Nov 2018 20:06:26 -0500 Subject: [PATCH 3/4] Serialize #[repr(packed)] structs by copying out their fields This assumes that #[repr(packed)] types are also Copy, which is a thing we can't verify in this context. At least we're in good company: the built-in #[derive]s all require #[derive(Copy)] for packed structs. Further discussion: https://github.com/m4b/scroll/issues/46#issuecomment-435052037 --- scroll_derive/src/lib.rs | 97 +++++++++++++++++++++++++++++++++------- 1 file changed, 80 insertions(+), 17 deletions(-) diff --git a/scroll_derive/src/lib.rs b/scroll_derive/src/lib.rs index d3ac09b..51c384b 100644 --- a/scroll_derive/src/lib.rs +++ b/scroll_derive/src/lib.rs @@ -8,6 +8,38 @@ extern crate syn; use proc_macro::TokenStream; +fn is_repr_packed(ast: &syn::DeriveInput) -> bool { + use proc_macro2::{Delimiter,TokenTree}; + + ast.attrs.iter() + .find(|attr| { + // find an Attribute with ident == "repr" + attr.path.segments.len() == 1 && + &attr.path.segments[0].ident.to_string() == "repr" + }) + .map(|repr| { + // if found, see if it's "(packed)" + // this doesn't seem ideal, but it does work + match repr.tts.clone().into_iter().next() { + Some(TokenTree::Group(group)) => { + if group.delimiter() == Delimiter::Parenthesis { + match group.stream().into_iter().next() { + Some(TokenTree::Ident(ident)) => { + &ident.to_string() == "packed" + } + _ => false, + } + } else { + false + } + }, + _ => false, + } + }) + // in the absence of #[repr], we're not packed + .unwrap_or(false) +} + fn impl_struct(name: &syn::Ident, fields: &syn::FieldsNamed) -> proc_macro2::TokenStream { let items: Vec<_> = fields.named.iter().map(|f| { let ident = &f.ident; @@ -70,21 +102,35 @@ pub fn derive_pread(input: TokenStream) -> TokenStream { gen.into() } -fn impl_try_into_ctx(name: &syn::Ident, fields: &syn::FieldsNamed) -> proc_macro2::TokenStream { +fn impl_try_into_ctx(name: &syn::Ident, fields: &syn::FieldsNamed, is_packed: bool) -> proc_macro2::TokenStream { let items: Vec<_> = fields.named.iter().map(|f| { let ident = &f.ident; let ty = &f.ty; match *ty { syn::Type::Array(_) => { - quote! { - for i in 0..self.#ident.len() { - dst.gwrite_with(&self.#ident[i], offset, ctx)?; + if is_packed { + quote! { + for i in 0..self.#ident.len() { + dst.gwrite_with(&{self.#ident[i]}, offset, ctx)?; + } + } + } else { + quote! { + for i in 0..self.#ident.len() { + dst.gwrite_with(&self.#ident[i], offset, ctx)?; + } } } }, _ => { - quote! { - dst.gwrite_with(&self.#ident, offset, ctx)? + if is_packed { + quote! { + dst.gwrite_with(&{self.#ident}, offset, ctx)? + } + } else { + quote! { + dst.gwrite_with(&self.#ident, offset, ctx)? + } } } } @@ -110,7 +156,7 @@ fn impl_pwrite(ast: &syn::DeriveInput) -> proc_macro2::TokenStream { syn::Data::Struct(ref data) => { match data.fields { syn::Fields::Named(ref fields) => { - impl_try_into_ctx(name, fields) + impl_try_into_ctx(name, fields, is_repr_packed(&ast)) }, _ => { panic!("Pwrite can only be derived for a regular struct with public fields") @@ -256,7 +302,7 @@ pub fn derive_ioread(input: TokenStream) -> TokenStream { gen.into() } -fn impl_into_ctx(name: &syn::Ident, fields: &syn::FieldsNamed) -> proc_macro2::TokenStream { +fn impl_into_ctx(name: &syn::Ident, fields: &syn::FieldsNamed, is_packed: bool) -> proc_macro2::TokenStream { let items: Vec<_> = fields.named.iter().map(|f| { let ident = &f.ident; let ty = &f.ty; @@ -264,18 +310,35 @@ fn impl_into_ctx(name: &syn::Ident, fields: &syn::FieldsNamed) -> proc_macro2::T match *ty { syn::Type::Array(ref array) => { let arrty = &array.elem; - quote! { - let size = ::scroll::export::mem::size_of::<#arrty>(); - for i in 0..self.#ident.len() { - dst.cwrite_with(&self.#ident[i], *offset, ctx); - *offset += size; + if is_packed { + quote! { + let size = ::scroll::export::mem::size_of::<#arrty>(); + for i in 0..self.#ident.len() { + dst.cwrite_with(&{self.#ident[i]}, *offset, ctx); + *offset += size; + } + } + } else { + quote! { + let size = ::scroll::export::mem::size_of::<#arrty>(); + for i in 0..self.#ident.len() { + dst.cwrite_with(&self.#ident[i], *offset, ctx); + *offset += size; + } } } }, _ => { - quote! { - dst.cwrite_with(&self.#ident, *offset, ctx); - *offset += #size; + if is_packed { + quote! { + dst.cwrite_with(&{self.#ident}, *offset, ctx); + *offset += #size; + } + } else { + quote! { + dst.cwrite_with(&self.#ident, *offset, ctx); + *offset += #size; + } } } } @@ -300,7 +363,7 @@ fn impl_iowrite(ast: &syn::DeriveInput) -> proc_macro2::TokenStream { syn::Data::Struct(ref data) => { match data.fields { syn::Fields::Named(ref fields) => { - impl_into_ctx(name, fields) + impl_into_ctx(name, fields, is_repr_packed(&ast)) }, _ => { panic!("IOwrite can only be derived for a regular struct with public fields") From efc16f36a2bb20a66887bdb818227a9ccb7162e2 Mon Sep 17 00:00:00 2001 From: Will Glynn Date: Thu, 1 Nov 2018 21:39:14 -0500 Subject: [PATCH 4/4] Change iowrite() to also take a reference --- README.md | 2 +- src/lesser.rs | 8 ++++---- src/lib.rs | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index c1630a1..6b087c4 100644 --- a/README.md +++ b/README.md @@ -163,7 +163,7 @@ fn write_io() -> Result<(), scroll::Error> { let mut bytes = [0x0u8; 10]; let mut cursor = Cursor::new(&mut bytes[..]); cursor.write_all(b"hello")?; - cursor.iowrite_with(0xdeadbeef as u32, BE)?; + cursor.iowrite_with(&(0xdeadbeef as u32), BE)?; assert_eq!(cursor.into_inner(), [0x68, 0x65, 0x6c, 0x6c, 0x6f, 0xde, 0xad, 0xbe, 0xef, 0x0]); Ok(()) } diff --git a/src/lesser.rs b/src/lesser.rs index f7283d4..e6af917 100644 --- a/src/lesser.rs +++ b/src/lesser.rs @@ -124,7 +124,7 @@ pub trait IOwrite: Write /// /// let mut bytes = [0x0u8; 4]; /// let mut bytes = Cursor::new(&mut bytes[..]); - /// bytes.iowrite(0xdeadbeef as u32).unwrap(); + /// bytes.iowrite(&(0xdeadbeef as u32)).unwrap(); /// /// #[cfg(target_endian = "little")] /// assert_eq!(bytes.into_inner(), [0xef, 0xbe, 0xad, 0xde,]); @@ -132,7 +132,7 @@ pub trait IOwrite: Write /// assert_eq!(bytes.into_inner(), [0xde, 0xad, 0xbe, 0xef,]); /// ``` #[inline] - fn iowrite + IntoCtx>(&mut self, n: N) -> Result<()> where Ctx: Default { + fn iowrite + IntoCtx>(&mut self, n: &N) -> Result<()> where Ctx: Default { let ctx = Ctx::default(); self.iowrite_with(n, ctx) } @@ -150,11 +150,11 @@ pub trait IOwrite: Write /// let mut bytes = [0x0u8; 10]; /// let mut cursor = Cursor::new(&mut bytes[..]); /// cursor.write_all(b"hello").unwrap(); - /// cursor.iowrite_with(0xdeadbeef as u32, BE).unwrap(); + /// cursor.iowrite_with(&(0xdeadbeef as u32), BE).unwrap(); /// assert_eq!(cursor.into_inner(), [0x68, 0x65, 0x6c, 0x6c, 0x6f, 0xde, 0xad, 0xbe, 0xef, 0x0]); /// ``` #[inline] - fn iowrite_with + IntoCtx>(&mut self, n: N, ctx: Ctx) -> Result<()> { + fn iowrite_with + IntoCtx>(&mut self, n: &N, ctx: Ctx) -> Result<()> { let mut buf = [0u8; 256]; let size = N::size_with(&ctx); let buf = &mut buf[0..size]; diff --git a/src/lib.rs b/src/lib.rs index 16b2561..9223334 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -92,7 +92,7 @@ //! let mut bytes = [0x0u8; 10]; //! let mut cursor = Cursor::new(&mut bytes[..]); //! cursor.write_all(b"hello").unwrap(); -//! cursor.iowrite_with(0xdeadbeef as u32, BE).unwrap(); +//! cursor.iowrite_with(&(0xdeadbeef as u32), BE).unwrap(); //! assert_eq!(cursor.into_inner(), [0x68, 0x65, 0x6c, 0x6c, 0x6f, 0xde, 0xad, 0xbe, 0xef, 0x0]); //! ``` //!