Repository: sharksforarms/deku (rust)

Issue:
DekuContainerWrite::to_slice returns the incorrect number of bytes written as of 0.19.1
Deku now incorrectly calculates the number of bits written, and this error is cumulative. This can be seen when you compare the difference between the size of `to_bytes()` and the return value of `to_slice()` - this was fine in 0.18.1. Unsure about 0.18.0.

Repro cases:

<details>
<summary>Test code being used for all (with variations for fields)</summary>

```rs
#[cfg(test)]
mod test {
    use super::*;
    use deku::DekuContainerWrite;

    #[test]
    fn test_encode_size_eq() {
        let data = Test {
            a: 7,
            b: 1,
            c: 3,
        };

        let d = data.to_bytes().unwrap();
        let mut dd = vec![0u8; d.len()];
        let written = data.to_slice(&mut dd).unwrap();
        dbg!(&*dd);
        assert_eq!(d, dd, "Data does not match");
        assert_eq!(written, d.len(), "Size does not match");
    }
}
```
</details>

Works:
```rs
#[derive(DekuWrite)]
pub struct Test {
    a: u8,
}

#[derive(DekuWrite)]
pub struct Test {
    #[deku(bits = 3)]
    a: u8,
}

#[derive(DekuWrite)]
pub struct Test {
    #[deku(bits = 8)]
    a: u8,
    b: u8
}

#[derive(DekuWrite)]
pub struct Test {
    #[deku(bits = 3)]
    a: u8,
    #[deku(bits = 3)]
    b: u8,
}
```

Broken:
```rs
// Reports as 1 byte written (not 2)
#[derive(DekuWrite)]
pub struct Test {
    #[deku(bits = 8)]
    a: u8,
    #[deku(bits = 8)]
    b: u8,
}

// Reports as 1 byte written (not 2)
#[derive(DekuWrite)]
pub struct Test {
    #[deku(bits = 3)]
    a: u8,
    b: u8,
}

// Reports as 1 byte written (not 2)
#[derive(DekuWrite)]
pub struct Test {
    #[deku(bits = 3)]
    a: u8,
    #[deku(bits = 7)]
    b: u8,
}

// Reports as 1 byte written (not 3)
#[derive(DekuWrite)]
pub struct Test {
    #[deku(bits = 3)]
    a: u8,
    #[deku(bits = 6)]
    b: u8,
    c: u8,
}
```

<details>
<summary>Extra info with logs, for help debugging</summary>

This was with a struct shaped like
```rs
#[derive(DekuWrite)]
pub struct Test {
    #[deku(bits = 3)]
    a: u8,
    #[deku(bits = 7)]
    b: u8,
    c: u8,
}
```
and initialized as
```rs
Test {
    a: 0b011,
    b: 0b11010,
    c: 0b01010110,
};
```
to try and get a distinct bit pattern:

```
Writing: Test.a
attempting 3 bits : [0, 1, 1]
no write: msb pre-pending 3 bits : [0, 1, 1] => [0, 1, 1]
Writing: Test.b
attempting 7 bits : [0, 0, 1, 1, 0, 1, 0]
msb pre-pending 3 bits : [0, 1, 1]
now 10 bits : [0, 1, 1, 0, 0, 1, 1, 0, 1, 0]
msb: wrote 8 bits : 0x[66]
leftover 2 bits : [1, 0]
Writing: Test.c
writing 1 bytes
leftover exists
attempting 8 bits : [0, 1, 0, 1, 0, 1, 1, 0]
msb pre-pending 2 bits : [1, 0]
now 10 bits : [1, 0, 0, 1, 0, 1, 0, 1, 1, 0]
msb: wrote 8 bits : 0x[95]
leftover 2 bits : [1, 0]
finalized: 2 bits leftover
finalized: wrote 8 bits
```

data is [102, 149, 128]
Length is reported as 1, to vec reports as 3.

</details>

This error accumulates with more items. Using separate enums with defined sizes vs attributes on members doesn't affect the result.
The data is actually written correctly - even in my own production code (reports as 21 bytes written when 28 were actually written) - but the reported written size is mismatched.

Pull request:
Fix writer dropping count of bytes written for unaligned writes
Fixes #560

Patch:
diff --git a/src/impls/cstring.rs b/src/impls/cstring.rs
index 606c70c..0a3292d 100644
--- a/src/impls/cstring.rs
+++ b/src/impls/cstring.rs
@@ -21,16 +21,14 @@ where
     }
 }
 
-impl<'a, Ctx: Copy> DekuReader<'a, Ctx> for CString
-where
-    u8: DekuReader<'a, Ctx>,
-{
+impl DekuReader<'_> for CString {
     fn from_reader_with_ctx<R: Read + Seek>(
         reader: &mut Reader<R>,
-        inner_ctx: Ctx,
+        _: (),
     ) -> Result<Self, DekuError> {
         let bytes =
-            Vec::from_reader_with_ctx(reader, (Limit::from(|b: &u8| *b == 0x00), inner_ctx))?;
+            Vec::<u8>::from_reader_with_ctx(reader, (Limit::from(|b: &u8| *b == 0x00), ()))?;
+
         let value = CString::from_vec_with_nul(bytes).map_err(|e| {
             DekuError::Parse(Cow::from(format!("Failed to convert Vec to CString: {e}")))
         })?;
@@ -39,15 +37,15 @@ where
     }
 }
 
-impl<'a, Ctx: Copy> DekuReader<'a, (ByteSize, Ctx)> for CString
+impl<'a> DekuReader<'a, ByteSize> for CString
 where
-    u8: DekuReader<'a, Ctx>,
+    u8: DekuReader<'a>,
 {
     fn from_reader_with_ctx<R: Read + Seek>(
         reader: &mut Reader<R>,
-        (byte_size, inner_ctx): (ByteSize, Ctx),
+        byte_size: ByteSize,
     ) -> Result<Self, DekuError> {
-        let bytes = Vec::from_reader_with_ctx(reader, (Limit::from(byte_size.0), inner_ctx))?;
+        let bytes = Vec::from_reader_with_ctx(reader, (Limit::from(byte_size.0), ()))?;
 
         let value = CString::from_vec_with_nul(bytes).map_err(|e| {
             DekuError::Parse(Cow::from(format!("Failed to convert Vec to CString: {e}")))
@@ -115,7 +113,7 @@ mod tests {
         let mut cursor = Cursor::new(input);
         let mut reader = Reader::new(&mut cursor);
         let res_read = if let Some(len) = len {
-            CString::from_reader_with_ctx(&mut reader, (ByteSize(len), ())).unwrap()
+            CString::from_reader_with_ctx(&mut reader, ByteSize(len)).unwrap()
         } else {
             CString::from_reader_with_ctx(&mut reader, ()).unwrap()
         };
diff --git a/src/writer.rs b/src/writer.rs
index 0fa7ac8..1fde9bc 100644
--- a/src/writer.rs
+++ b/src/writer.rs
@@ -154,7 +154,7 @@ impl<W: Write + Seek> Writer<W> {
             bits = unsafe { bits.get_unchecked(count * bits_of::<u8>()..) };
 
             // TODO: with_capacity?
-            self.bits_written = buf.len() * 8;
+            self.bits_written += buf.len() * 8;
             self.leftover = (bits.to_bitvec(), order);
             if let Err(e) = self.inner.write_all(&buf) {
                 return Err(DekuError::Io(e.kind()));
@@ -193,7 +193,7 @@ impl<W: Write + Seek> Writer<W> {
                 return Err(DekuError::Io(e.kind()));
             }
 
-            self.bits_written = buf.len() * 8;
+            self.bits_written += buf.len() * 8;
             self.leftover = (bits.to_bitvec(), order);
 
             #[cfg(feature = "logging")]
@@ -281,7 +281,7 @@ impl<W: Write + Seek> Writer<W> {
             #[cfg(feature = "logging")]
             log::trace!("finalized: wrote {} bits", buf.len() * 8);
 
-            self.bits_written = buf.len() * 8;
+            self.bits_written += buf.len() * 8;
         }
         Ok(())
     }


Test patch:
diff --git a/tests/test_cstring.rs b/tests/test_cstring.rs
index 10d7636..ede6e62 100644
--- a/tests/test_cstring.rs
+++ b/tests/test_cstring.rs
@@ -27,3 +27,32 @@ fn test_cstring_bytes() {
     let (_, d) = Data::from_bytes((bytes, 0)).unwrap();
     assert_eq!(d.s, CString::new("test").unwrap());
 }
+
+#[test]
+fn test_cstring_valid_bytes() {
+    #[derive(PartialEq, Debug, DekuRead, DekuWrite)]
+    pub struct Data {
+        #[deku(bytes = "5")]
+        s: CString,
+    }
+
+    let bytes = &[b't', b'e', b's', b't', b'\0'];
+    let (_, d) = Data::from_bytes((bytes, 0)).unwrap();
+    assert_eq!(d.s, CString::new("test").unwrap());
+}
+
+#[should_panic(
+    expected = "Failed to convert Vec to CString: data provided contains an interior nul byte at pos 4"
+)]
+#[test]
+fn test_cstring_trailing_nul() {
+    #[derive(PartialEq, Debug, DekuRead, DekuWrite)]
+    pub struct Data {
+        #[deku(bytes = "6")]
+        s: CString,
+    }
+
+    let bytes = &[b't', b'e', b's', b't', b'\0', b'\0'];
+    let (_, d) = Data::from_bytes((bytes, 0)).unwrap();
+    assert_eq!(d.s, CString::new("test").unwrap());
+}
diff --git a/tests/test_to_slice.rs b/tests/test_to_slice.rs
index e54b4c9..1064dee 100644
--- a/tests/test_to_slice.rs
+++ b/tests/test_to_slice.rs
@@ -47,3 +47,53 @@ fn test_to_slice_panic_writer_failure() {
     let mut out = [0x00; 2];
     a.to_slice(&mut out).unwrap();
 }
+
+#[cfg(feature = "bits")]
+#[derive(PartialEq, Debug, DekuRead, DekuWrite)]
+pub struct C {
+    a: u8,
+    #[deku(bits = "3", endian = "little")]
+    b: u8,
+    c: u8,
+    #[deku(bits = "13", endian = "little")]
+    d: u16,
+    e: u8,
+}
+
+#[test]
+#[cfg(feature = "bits")]
+fn test_to_slice_counts_unaligned_writes_with_aligned_end() {
+    let bytes = [0x11, 0x22, 0x33, 0x44, 0x55];
+    let (leftover, c) = C::from_bytes((&bytes, 0)).unwrap();
+    assert_eq!(leftover, (&[][..], 0));
+
+    let mut out = [0x00; 5];
+    let amt_written = c.to_slice(&mut out).unwrap();
+    assert_eq!(bytes, out);
+    assert_eq!(bytes.len(), amt_written);
+}
+
+#[cfg(feature = "bits")]
+#[derive(PartialEq, Debug, DekuRead, DekuWrite)]
+pub struct D {
+    a: u8,
+    #[deku(bits = "3", endian = "little")]
+    b: u8,
+    c: u8,
+    #[deku(bits = "12", endian = "little")]
+    d: u16,
+    e: u8,
+}
+
+#[test]
+#[cfg(feature = "bits")]
+fn test_to_slice_counts_unaligned_writes_with_unaligned_end() {
+    let bytes = [0x11, 0x22, 0x33, 0x44, 0x54];
+    let (leftover, d) = D::from_bytes((&bytes, 0)).unwrap();
+    assert_eq!(leftover, (&[0x54][..], 7));
+
+    let mut out = [0x00; 5];
+    let amt_written = d.to_slice(&mut out).unwrap();
+    assert_eq!(bytes, out);
+    assert_eq!(bytes.len(), amt_written);
+}