From bd17d606e82b7b6c891fa90d071ffcaec979ff0f Mon Sep 17 00:00:00 2001
From: Uli Schlachter <psychon@znc.in>
Date: Fri, 6 Oct 2023 17:14:14 +0200
Subject: [PATCH] Fix packet reader test

Clippy nightly has a new warning for us:

    error: this loop never actually loops
         --> x11rb-protocol/src/packet_reader.rs:135:9
          |
      135 | /         for mut packet in packets {
      136 | |             let original_packet = packet.clone();
      137 | |
      138 | |             loop {
      ...   |
      148 | |             }
      149 | |         }
          | |_________^
          |
          = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#never_loop
          = note: `#[deny(clippy::never_loop)]` on by default
      help: if you need the first element of the iterator, try writing
          |
      135 |         if let Some(mut packet) = packets.into_iter().next() {
          |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

It turns out that the function test_packets(), which is supposed to test
that a list of packets is correctly read by PacketReader, only ever
tested the first packet and ignored everything else.

This commit fixes the test.

First, test_packets() now puts all the packet data into one, big chunk,
instead of feeding the packets already on the right boundaries to the
PacketReader. After all, we want to test that this correctly determines
the boundaries again.

After fixing that, the test started to fail. It turns out that two of
the tests actually produced packages whose length is not a multiple of
four, but all X11 packets must have a length that is a multiple of four.
Fix this by rounding down to a correct multiple.

These tests were added in commit 2082b2b788a96. Before that, there were
no tests at all.

Signed-off-by: Uli Schlachter <psychon@znc.in>
---
 x11rb-protocol/src/packet_reader.rs | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/x11rb-protocol/src/packet_reader.rs b/x11rb-protocol/src/packet_reader.rs
index 2c5f87cd..5c1ee063 100644
--- a/x11rb-protocol/src/packet_reader.rs
+++ b/x11rb-protocol/src/packet_reader.rs
@@ -131,19 +131,21 @@ mod tests {
     use alloc::{vec, vec::Vec};
 
     fn test_packets(packets: Vec<Vec<u8>>) {
-        let mut reader = PacketReader::default();
-        for mut packet in packets {
-            let original_packet = packet.clone();
+        // Combine all packet data into one big chunk and test that the packet reader splits things
+        let mut all_data = packets.iter().flatten().copied().collect::<Vec<u8>>();
 
+        let mut reader = PacketReader::default();
+        for (i, packet) in packets.into_iter().enumerate() {
+            std::println!("Checking packet {i}");
             loop {
                 let buffer = reader.buffer();
-                let amount = std::cmp::min(buffer.len(), packet.len());
-                buffer.copy_from_slice(&packet[..amount]);
-                let _ = packet.drain(..amount);
+                let amount = std::cmp::min(buffer.len(), all_data.len());
+                buffer.copy_from_slice(&all_data[..amount]);
+                let _ = all_data.drain(..amount);
 
                 if let Some(read_packet) = reader.advance(amount) {
-                    assert_eq!(read_packet, original_packet);
-                    return;
+                    assert_eq!(read_packet, packet);
+                    break;
                 }
             }
         }
@@ -193,9 +195,12 @@ mod tests {
             let variation = ((i - 50) * (i - 50)) as f32;
             let variation = -1.0 / 25.0 * variation + 100.0;
             let variation = variation as usize;
+            // round to a multiple of 4
+            let variation = variation / 4 * 4;
 
             let mut len = 1200 + variation;
             let mut packet = vec![0; len];
+            assert_eq!(0, len % 4);
             len = (len - 32) / 4;
 
             // write "len" to bytes 4..8 in the packet
@@ -219,11 +224,14 @@ mod tests {
                 let variation = ((i - 50) * (i - 50)) as f32;
                 let variation = -1.0 / 25.0 * variation + 100.0;
                 let variation = variation as usize;
+                // round to a multiple of 4
+                let variation = variation / 4 * 4;
 
                 1200 + variation
             } else {
                 32
             };
+            assert_eq!(0, len % 4);
             let mut packet = vec![0; len];
             len = (len - 32) / 4;