Repository: sharksforarms/deku
Pull request #124 (2020-10-29 09:18:55)

Implement attributes for the new container limit kinds
This PR implements the attributes for the new limit kinds introduced in the previous PR (#118), taking care of issues #76 and #110.

For #76, I'm conscious of the fact that a decision around the specific attrbute was never reached so I've arbitrarily opted for `bits_read` and `bytes_read` to avoid overloading any existing attributes, though I'm more than happy to change this if we decide something else is better.




sharksforarms: Small comments! Looks good to me!

Naming is hard.
samuelsleight: > Naming is hard.

Oh man, this is certainly true 😛  I really wish I could come up with something better for these bits/bytes attributes

Patch:
diff --git a/deku-derive/src/lib.rs b/deku-derive/src/lib.rs
index c1eb4bf..302c475 100644
--- a/deku-derive/src/lib.rs
+++ b/deku-derive/src/lib.rs
@@ -205,6 +205,12 @@ struct FieldData {
     /// tokens providing the length of the container
     count: Option<TokenStream>,
 
+    /// tokens providing the number of bits for the length of the container
+    bits_read: Option<TokenStream>,
+
+    /// a predicate to decide when to stop reading elements into the container
+    until: Option<TokenStream>,
+
     /// apply a function to the field after it's read
     map: Option<TokenStream>,
 
@@ -237,6 +243,11 @@ impl FieldData {
 
         let bits = receiver.bytes.map(|b| b * 8).or(receiver.bits);
 
+        let bits_read = receiver
+            .bytes_read
+            .map(|tokens| quote! { (#tokens) * 8 })
+            .or(receiver.bits_read);
+
         let default = receiver.default.unwrap_or(quote! { Default::default() });
 
         let ctx = receiver
@@ -251,6 +262,8 @@ impl FieldData {
             endian: receiver.endian,
             bits,
             count: receiver.count,
+            bits_read,
+            until: receiver.until,
             map: receiver.map,
             ctx,
             update: receiver.update,
@@ -460,6 +473,18 @@ struct DekuFieldReceiver {
     #[darling(default, map = "option_as_tokenstream")]
     count: Option<TokenStream>,
 
+    /// tokens providing the number of bits for the length of the container
+    #[darling(default, map = "option_as_tokenstream")]
+    bits_read: Option<TokenStream>,
+
+    /// tokens providing the number of bytes for the length of the container
+    #[darling(default, map = "option_as_tokenstream")]
+    bytes_read: Option<TokenStream>,
+
+    /// a predicate to decide when to stop reading elements into the container
+    #[darling(default, map = "option_as_tokenstream")]
+    until: Option<TokenStream>,
+
     /// apply a function to the field after it's read
     #[darling(default, map = "option_as_tokenstream")]
     map: Option<TokenStream>,
diff --git a/deku-derive/src/macros/deku_read.rs b/deku-derive/src/macros/deku_read.rs
index e6725fa..db6b470 100644
--- a/deku-derive/src/macros/deku_read.rs
+++ b/deku-derive/src/macros/deku_read.rs
@@ -348,20 +348,31 @@ fn emit_field_read(
     } else {
         let read_args = gen_field_args(field_endian, f.bits, f.ctx.as_ref())?;
 
-        // Count is special, we need to generate `(count, (other, ..))` for it.
+        // The container limiting options are special, we need to generate `(limit, (other, ..))` for them.
+        // These have a problem where when it isn't a copy type, the field will be moved.
+        // e.g. struct FooBar {
+        //   a: Baz // a type implement `Into<usize>` but not `Copy`.
+        //   #[deku(count = "a") <-- Oops, use of moved value: `a`
+        //   b: Vec<_>
+        // }
         if let Some(field_count) = &f.count {
-            // The count has same problem, when it isn't a copy type, the field will be moved.
-            // e.g. struct FooBar {
-            //   a: Baz // a type implement `Into<usize>` but not `Copy`.
-            //   #[deku(count = "a") <-- Oops, use of moved value: `a`
-            //   b: Vec<_>
-            // }
             quote! {
                 {
                     use core::borrow::Borrow;
-                    DekuRead::read(rest, (usize::try_from(*((#field_count).borrow()))?.into(), (#read_args)))
+                    DekuRead::read(rest, (deku::ctx::Limit::new_count(usize::try_from(*((#field_count).borrow()))?), (#read_args)))
                 }
             }
+        } else if let Some(field_bits) = &f.bits_read {
+            quote! {
+                {
+                    use core::borrow::Borrow;
+                    DekuRead::read(rest, (deku::ctx::Limit::new_bits(deku::ctx::BitSize(usize::try_from(*((#field_bits).borrow()))?)), (#read_args)))
+                }
+            }
+        } else if let Some(field_until) = &f.until {
+            // We wrap the input into another closure here to enforce that it is actually a callable
+            // Otherwise, an incorrectly passed-in integer could unexpectedly convert into a `Count` limit
+            quote! {DekuRead::read(rest, (deku::ctx::Limit::new_until(#field_until), (#read_args)))}
         } else {
             quote! {DekuRead::read(rest, (#read_args))}
         }
diff --git a/src/attributes.rs b/src/attributes.rs
index 1e7cb5f..ccdb8d0 100644
--- a/src/attributes.rs
+++ b/src/attributes.rs
@@ -10,6 +10,9 @@ A documentation-only module for #\[deku\] attributes
 | [bits](#bits) | field | Set the bit-size of the field
 | [bytes](#bytes) | field | Set the byte-size of the field
 | [count](#count) | field | Set the field representing the element count of a container
+| [bits_read](#bits_read) | field | Set the field representing the number of bits to read into a container
+| [bytes_read](#bytes_read) | field | Set the field representing the number of bytes to read into a container
+| [until](#until) | field | Set a predicate returning when to stop reading elements into a container
 | [update](#update) | field | Apply code over the field when `.update()` is called
 | [skip](#skip) | field | Skip the reading/writing of a field
 | [cond](#cond) | field | Conditional expression for the field
@@ -234,6 +237,88 @@ assert_eq!(data, value);
 
 **Note**: See [update](#update) for more information on the attribute!
 
+
+# bytes_read
+
+Specify the field representing the total number of bytes to read into a container
+
+See the following example, where `InnerDekuTest` is 2 bytes, so setting `bytes_read` to
+4 will read 2 items into the container:
+```rust
+# use deku::prelude::*;
+# use std::convert::{TryInto, TryFrom};
+# #[derive(Debug, PartialEq, DekuRead, DekuWrite)]
+struct InnerDekuTest {
+    field_a: u8,
+    field_b: u8
+}
+
+# #[derive(Debug, PartialEq, DekuRead, DekuWrite)]
+struct DekuTest {
+    #[deku(update = "(self.items.len() / 2)")]
+    bytes: u8,
+
+    #[deku(bytes_read = "bytes")]
+    items: Vec<InnerDekuTest>,
+}
+
+let data: Vec<u8> = vec![0x04, 0xAB, 0xBC, 0xDE, 0xEF];
+
+let value = DekuTest::try_from(data.as_ref()).unwrap();
+
+assert_eq!(
+    DekuTest {
+       bytes: 0x04,
+       items: vec![
+           InnerDekuTest{field_a: 0xAB, field_b: 0xBC},
+           InnerDekuTest{field_a: 0xDE, field_b: 0xEF}],
+    },
+    value
+);
+
+let value: Vec<u8> = value.try_into().unwrap();
+assert_eq!(data, value);
+```
+
+**Note**: See [update](#update) for more information on the attribute!
+
+
+# bits_read
+
+This is equivalent to [bytes_read](#bytes_read), however specifies the bit limit instead
+of a byte limit
+
+
+# until
+
+Specifies a predicate which sets when to stop reading values into the container.
+
+The predicate is given a borrow to each item as it is read, and must return a boolean
+as to whether this should be the last item or not. If it returns true, then reading stops.
+
+A good example of this is to read a null-terminated string:
+```rust
+# use deku::prelude::*;
+# use std::convert::{TryInto, TryFrom};
+# use std::ffi::CString;
+# #[derive(Debug, PartialEq, DekuRead)]
+struct DekuTest {
+    #[deku(until = "|v: &u8| *v == 0")]
+    string: Vec<u8>
+}
+
+let data: Vec<u8> = vec![72, 101, 108, 108, 111, 0];
+let value = DekuTest::try_from(data.as_ref()).unwrap();
+
+assert_eq!(
+    DekuTest {
+        string: CString::new(b"Hello".to_vec()).unwrap().into_bytes_with_nul()
+    },
+    value
+);
+```
+
+
 # update
 
 Specify custom code to run on the field when `.update()` is called on the struct/enum
diff --git a/src/ctx.rs b/src/ctx.rs
index 8eb8112..59a2399 100644
--- a/src/ctx.rs
+++ b/src/ctx.rs
@@ -104,6 +104,27 @@ impl<T> From<BitSize> for Limit<T, fn(&T) -> bool> {
     }
 }
 
+impl<T, Predicate: for<'a> FnMut(&'a T) -> bool> Limit<T, Predicate> {
+    /// Constructs a new Limit that reads until the given predicate returns true
+    /// The predicate is given a reference to the latest read value and must return
+    /// true to stop reading
+    pub fn new_until(predicate: Predicate) -> Self {
+        predicate.into()
+    }
+}
+
+impl<T> Limit<T, fn(&T) -> bool> {
+    /// Constructs a new Limit that reads unil the given number of elements are read
+    pub fn new_count(count: usize) -> Self {
+        count.into()
+    }
+
+    /// Constructs a new Limit that reads until the given number of bits have been read
+    pub fn new_bits(bits: BitSize) -> Self {
+        bits.into()
+    }
+}
+
 /// The number bits in a field
 #[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd)]
 pub struct BitSize(pub usize);


Test patch:
diff --git a/tests/test_attributes/mod.rs b/tests/test_attributes/mod.rs
index 3f433ba..5d68916 100644
--- a/tests/test_attributes/mod.rs
+++ b/tests/test_attributes/mod.rs
@@ -1,6 +1,6 @@
 mod test_cond;
-mod test_count;
 mod test_ctx;
 mod test_map;
 mod test_skip;
 mod test_update;
+mod test_vec_limits;
diff --git a/tests/test_attributes/test_vec_limits/mod.rs b/tests/test_attributes/test_vec_limits/mod.rs
new file mode 100644
index 0000000..48aa212
--- /dev/null
+++ b/tests/test_attributes/test_vec_limits/mod.rs
@@ -0,0 +1,4 @@
+mod test_bits_read;
+mod test_bytes_read;
+mod test_count;
+mod test_until;
diff --git a/tests/test_attributes/test_vec_limits/test_bits_read.rs b/tests/test_attributes/test_vec_limits/test_bits_read.rs
new file mode 100644
index 0000000..90e4707
--- /dev/null
+++ b/tests/test_attributes/test_vec_limits/test_bits_read.rs
@@ -0,0 +1,78 @@
+use deku::prelude::*;
+use std::convert::{TryFrom, TryInto};
+
+#[test]
+fn test_bits_read_static() {
+    #[derive(PartialEq, Debug, DekuRead, DekuWrite)]
+    struct TestStruct {
+        #[deku(endian = "little", bits_read = "16")]
+        data: Vec<u16>,
+    }
+
+    let test_data: Vec<u8> = [0xAA, 0xBB].to_vec();
+
+    let mut ret_read = TestStruct::try_from(test_data.as_ref()).unwrap();
+    assert_eq!(
+        TestStruct {
+            // We should read 16 bits, not 16 elements,
+            // thus resulting in a single u16 element
+            data: vec![0xBBAA]
+        },
+        ret_read
+    );
+
+    // Add an item to the vec
+    ret_read.data.push(0xFFEE);
+
+    let ret_write: Vec<u8> = ret_read.try_into().unwrap();
+    assert_eq!([0xAA, 0xBB, 0xEE, 0xFF].to_vec(), ret_write);
+}
+
+#[test]
+fn test_bits_read_from_field() {
+    #[derive(PartialEq, Debug, DekuRead, DekuWrite)]
+    struct TestStruct {
+        bits: u8,
+
+        #[deku(endian = "little", bits_read = "bits")]
+        data: Vec<u16>,
+    }
+
+    let test_data: Vec<u8> = [16, 0xAA, 0xBB].to_vec();
+
+    let mut ret_read = TestStruct::try_from(test_data.as_ref()).unwrap();
+    assert_eq!(
+        TestStruct {
+            bits: 16,
+
+            // We should read 16 bits, not 16 elements,
+            // thus resulting in a single u16 element
+            data: vec![0xBBAA]
+        },
+        ret_read
+    );
+
+    // Add an item to the vec
+    ret_read.data.push(0xFFEE);
+
+    // `bits` is still 16, this is intended. `update` attribute should be
+    // used if `bits` is to be updated
+    let ret_write: Vec<u8> = ret_read.try_into().unwrap();
+    assert_eq!([16, 0xAA, 0xBB, 0xEE, 0xFF].to_vec(), ret_write);
+}
+
+#[test]
+#[should_panic(expected = "Parse(\"not enough data: expected 16 bits got 0 bits\")")]
+fn test_bits_read_error() {
+    #[derive(PartialEq, Debug, DekuRead, DekuWrite)]
+    struct TestStruct {
+        bits: u8,
+
+        #[deku(endian = "little", bits_read = "bits")]
+        data: Vec<u16>,
+    }
+
+    let test_data: Vec<u8> = [17, 0xAA, 0xBB].to_vec();
+
+    let _ret_read = TestStruct::try_from(test_data.as_ref()).unwrap();
+}
diff --git a/tests/test_attributes/test_vec_limits/test_bytes_read.rs b/tests/test_attributes/test_vec_limits/test_bytes_read.rs
new file mode 100644
index 0000000..0a7317c
--- /dev/null
+++ b/tests/test_attributes/test_vec_limits/test_bytes_read.rs
@@ -0,0 +1,78 @@
+use deku::prelude::*;
+use std::convert::{TryFrom, TryInto};
+
+#[test]
+fn test_bytes_read_static() {
+    #[derive(PartialEq, Debug, DekuRead, DekuWrite)]
+    struct TestStruct {
+        #[deku(endian = "little", bytes_read = "2")]
+        data: Vec<u16>,
+    }
+
+    let test_data: Vec<u8> = [0xAA, 0xBB].to_vec();
+
+    let mut ret_read = TestStruct::try_from(test_data.as_ref()).unwrap();
+    assert_eq!(
+        TestStruct {
+            // We should read two bytes, not two elements,
+            // thus resulting in a single u16 element
+            data: vec![0xBBAA]
+        },
+        ret_read
+    );
+
+    // Add an item to the vec
+    ret_read.data.push(0xFFEE);
+
+    let ret_write: Vec<u8> = ret_read.try_into().unwrap();
+    assert_eq!([0xAA, 0xBB, 0xEE, 0xFF].to_vec(), ret_write);
+}
+
+#[test]
+fn test_bytes_read_from_field() {
+    #[derive(PartialEq, Debug, DekuRead, DekuWrite)]
+    struct TestStruct {
+        bytes: u8,
+
+        #[deku(endian = "little", bytes_read = "bytes")]
+        data: Vec<u16>,
+    }
+
+    let test_data: Vec<u8> = [0x02, 0xAA, 0xBB].to_vec();
+
+    let mut ret_read = TestStruct::try_from(test_data.as_ref()).unwrap();
+    assert_eq!(
+        TestStruct {
+            bytes: 0x02,
+
+            // We should read two bytes, not two elements,
+            // thus resulting in a single u16 element
+            data: vec![0xBBAA]
+        },
+        ret_read
+    );
+
+    // Add an item to the vec
+    ret_read.data.push(0xFFEE);
+
+    // `bytes` is still 0x02, this is intended. `update` attribute should be
+    // used if `bytes` is to be updated
+    let ret_write: Vec<u8> = ret_read.try_into().unwrap();
+    assert_eq!([0x02, 0xAA, 0xBB, 0xEE, 0xFF].to_vec(), ret_write);
+}
+
+#[test]
+#[should_panic(expected = "Parse(\"not enough data: expected 16 bits got 0 bits\")")]
+fn test_bytes_read_error() {
+    #[derive(PartialEq, Debug, DekuRead, DekuWrite)]
+    struct TestStruct {
+        bytes: u8,
+
+        #[deku(endian = "little", bytes_read = "bytes")]
+        data: Vec<u16>,
+    }
+
+    let test_data: Vec<u8> = [0x03, 0xAA, 0xBB].to_vec();
+
+    let _ret_read = TestStruct::try_from(test_data.as_ref()).unwrap();
+}
diff --git a/tests/test_attributes/test_count.rs b/tests/test_attributes/test_vec_limits/test_count.rs
similarity index 100%
rename from tests/test_attributes/test_count.rs
rename to tests/test_attributes/test_vec_limits/test_count.rs
diff --git a/tests/test_attributes/test_vec_limits/test_until.rs b/tests/test_attributes/test_vec_limits/test_until.rs
new file mode 100644
index 0000000..a182e7b
--- /dev/null
+++ b/tests/test_attributes/test_vec_limits/test_until.rs
@@ -0,0 +1,73 @@
+use deku::prelude::*;
+use std::convert::{TryFrom, TryInto};
+
+#[test]
+fn test_until_static() {
+    #[derive(PartialEq, Debug, DekuRead, DekuWrite)]
+    struct TestStruct {
+        #[deku(until = "|v: &u8| *v == 0xBB")]
+        data: Vec<u8>,
+    }
+
+    let test_data: Vec<u8> = [0xAA, 0xBB].to_vec();
+
+    let mut ret_read = TestStruct::try_from(test_data.as_ref()).unwrap();
+    assert_eq!(
+        TestStruct {
+            data: vec![0xAA, 0xBB]
+        },
+        ret_read
+    );
+
+    // Add an item to the vec
+    ret_read.data.push(0xFF);
+
+    let ret_write: Vec<u8> = ret_read.try_into().unwrap();
+    assert_eq!([0xAA, 0xBB, 0xFF].to_vec(), ret_write);
+}
+
+#[test]
+fn test_until_from_field() {
+    #[derive(PartialEq, Debug, DekuRead, DekuWrite)]
+    struct TestStruct {
+        until: u8,
+
+        #[deku(until = "|v: &u8| *v == *until")]
+        data: Vec<u8>,
+    }
+
+    let test_data: Vec<u8> = [0xBB, 0xAA, 0xBB].to_vec();
+
+    let mut ret_read = TestStruct::try_from(test_data.as_ref()).unwrap();
+    assert_eq!(
+        TestStruct {
+            until: 0xBB,
+            data: vec![0xAA, 0xBB]
+        },
+        ret_read
+    );
+
+    // Add an item to the vec
+    ret_read.data.push(0xFF);
+
+    // `until` is still 0x02, this is intended. `update` attribute should be
+    // used if `until` is to be updated
+    let ret_write: Vec<u8> = ret_read.try_into().unwrap();
+    assert_eq!([0xBB, 0xAA, 0xBB, 0xFF].to_vec(), ret_write);
+}
+
+#[test]
+#[should_panic(expected = "Parse(\"not enough data: expected 8 bits got 0 bits\")")]
+fn test_until_error() {
+    #[derive(PartialEq, Debug, DekuRead, DekuWrite)]
+    struct TestStruct {
+        until: u8,
+
+        #[deku(until = "|v: &u8| *v == *until")]
+        data: Vec<u8>,
+    }
+
+    let test_data: Vec<u8> = [0xCC, 0xAA, 0xBB].to_vec();
+
+    let _ret_read = TestStruct::try_from(test_data.as_ref()).unwrap();
+}