[Test code] Add try_from()/unwrap() for test code

JBYoshi · JonathanWoollett-Light · commit af83175e6e98 · 2023-10-09T18:31:31.000+01:00
Some places in the code don't allow us to re-type values to let the
compiler verify that those conversions are safe, particularly with
length values. Since these are for test code, I don't expect overflows
to happen, so I've just inserted unwraps here.

Signed-off-by: Jonathan Browne &lt;12983479+JBYoshi@users.noreply.github.com&gt;
diff --git a/src/seccompiler/src/backend.rs b/src/seccompiler/src/backend.rs
@@ -917,7 +917,7 @@ mod tests {
                 assert_eq!(rc, 0);
             }
             let bpf_prog = sock_fprog {
-                len: bpf_filter.len() as u16,
+                len: u16::try_from(bpf_filter.len()).unwrap(),
                 filter: bpf_filter.as_ptr(),
             };
             let bpf_prog_ptr = &bpf_prog as *const sock_fprog;
diff --git a/src/vmm/src/arch/x86_64/mod.rs b/src/vmm/src/arch/x86_64/mod.rs
@@ -292,7 +292,7 @@ mod tests {
 
         // Exercise the scenario where the field storing the length of the e820 entry table is
         // is bigger than the allocated memory.
-        params.e820_entries = params.e820_table.len() as u8 + 1;
+        params.e820_entries = u8::try_from(params.e820_table.len()).unwrap() + 1;
         assert!(add_e820_entry(
             &mut params,
             e820_map[0].addr,
diff --git a/src/vmm/src/devices/legacy/i8042.rs b/src/vmm/src/devices/legacy/i8042.rs
@@ -412,7 +412,7 @@ mod tests {
 
         // Test buffer full.
         for i in 0..BUF_SIZE {
-            i8042.push_byte(i as u8).unwrap();
+            i8042.push_byte(i.try_into().unwrap()).unwrap();
             assert_eq!(i8042.buf_len(), i + 1);
         }
         assert_eq!(
diff --git a/src/vmm/src/devices/virtio/balloon/device.rs b/src/vmm/src/devices/virtio/balloon/device.rs
@@ -853,7 +853,7 @@ pub(crate) mod tests {
                 &infq,
                 0,
                 page_addr,
-                SIZE_OF_U32 as u32,
+                SIZE_OF_U32.try_into().unwrap(),
                 VIRTQ_DESC_F_NEXT | VIRTQ_DESC_F_WRITE,
             );
 
@@ -873,7 +873,7 @@ pub(crate) mod tests {
                 &infq,
                 1,
                 page_addr,
-                SIZE_OF_U32 as u32 + 1,
+                u32::try_from(SIZE_OF_U32).unwrap() + 1,
                 VIRTQ_DESC_F_NEXT,
             );
 
@@ -908,7 +908,13 @@ pub(crate) mod tests {
         // to trigger the inflate event queue.
         {
             mem.write_obj::<u32>(0x1, GuestAddress(page_addr)).unwrap();
-            set_request(&infq, 0, page_addr, SIZE_OF_U32 as u32, VIRTQ_DESC_F_NEXT);
+            set_request(
+                &infq,
+                0,
+                page_addr,
+                SIZE_OF_U32.try_into().unwrap(),
+                VIRTQ_DESC_F_NEXT,
+            );
 
             check_metric_after_block!(
                 METRICS.balloon.event_fails,
@@ -929,7 +935,13 @@ pub(crate) mod tests {
         // Test the happy case.
         {
             mem.write_obj::<u32>(0x1, GuestAddress(page_addr)).unwrap();
-            set_request(&infq, 0, page_addr, SIZE_OF_U32 as u32, VIRTQ_DESC_F_NEXT);
+            set_request(
+                &infq,
+                0,
+                page_addr,
+                SIZE_OF_U32.try_into().unwrap(),
+                VIRTQ_DESC_F_NEXT,
+            );
 
             check_metric_after_block!(
                 METRICS.balloon.inflate_count,
@@ -957,7 +969,13 @@ pub(crate) mod tests {
 
         // Error case: forgot to trigger deflate event queue.
         {
-            set_request(&defq, 0, page_addr, SIZE_OF_U32 as u32, VIRTQ_DESC_F_NEXT);
+            set_request(
+                &defq,
+                0,
+                page_addr,
+                SIZE_OF_U32.try_into().unwrap(),
+                VIRTQ_DESC_F_NEXT,
+            );
             check_metric_after_block!(
                 METRICS.balloon.event_fails,
                 1,
@@ -971,7 +989,13 @@ pub(crate) mod tests {
 
         // Happy case.
         {
-            set_request(&defq, 1, page_addr, SIZE_OF_U32 as u32, VIRTQ_DESC_F_NEXT);
+            set_request(
+                &defq,
+                1,
+                page_addr,
+                SIZE_OF_U32.try_into().unwrap(),
+                VIRTQ_DESC_F_NEXT,
+            );
             check_metric_after_block!(
                 METRICS.balloon.deflate_count,
                 1,
@@ -993,7 +1017,13 @@ pub(crate) mod tests {
 
         // Error case: forgot to trigger stats event queue.
         {
-            set_request(&statsq, 0, 0x1000, SIZE_OF_STAT as u32, VIRTQ_DESC_F_NEXT);
+            set_request(
+                &statsq,
+                0,
+                0x1000,
+                SIZE_OF_STAT.try_into().unwrap(),
+                VIRTQ_DESC_F_NEXT,
+            );
             check_metric_after_block!(
                 METRICS.balloon.event_fails,
                 1,
@@ -1029,7 +1059,7 @@ pub(crate) mod tests {
                 &statsq,
                 0,
                 page_addr,
-                2 * SIZE_OF_STAT as u32,
+                2 * u32::try_from(SIZE_OF_STAT).unwrap(),
                 VIRTQ_DESC_F_NEXT,
             );
             check_metric_after_block!(METRICS.balloon.stats_updates_count, 1, {
diff --git a/src/vmm/src/devices/virtio/mmio.rs b/src/vmm/src/devices/virtio/mmio.rs
@@ -755,7 +755,7 @@ pub(crate) mod tests {
         let mut buf = vec![0; 4];
         let queue_len = d.locked_device().queues().len();
         for q in 0..queue_len {
-            d.queue_select = q as u32;
+            d.queue_select = q.try_into().unwrap();
             write_le_u32(&mut buf[..], 16);
             d.bus_write(0x38, &buf[..]);
             write_le_u32(&mut buf[..], 1);
@@ -808,7 +808,7 @@ pub(crate) mod tests {
         let mut buf = vec![0; 4];
         let queues_count = d.locked_device().queues().len();
         for q in 0..queues_count {
-            d.queue_select = q as u32;
+            d.queue_select = q.try_into().unwrap();
             write_le_u32(&mut buf[..], 16);
             d.bus_write(0x38, &buf[..]);
             write_le_u32(&mut buf[..], 1);
diff --git a/src/vmm/src/devices/virtio/net/device.rs b/src/vmm/src/devices/virtio/net/device.rs
@@ -1150,7 +1150,8 @@ pub mod tests {
         // Check that the frame wasn't deferred.
         assert!(!th.net().rx_deferred_frame);
         // Check that the frame has been written successfully to the valid Rx descriptor chain.
-        th.rxq.check_used_elem(3, 5, frame.len() as u32);
+        th.rxq
+            .check_used_elem(3, 5, frame.len().try_into().unwrap());
         th.rxq.dtable[5].check_data(&frame);
     }
 
@@ -1186,7 +1187,8 @@ pub mod tests {
         assert_eq!(th.rxq.used.idx.get(), 1);
         assert!(&th.net().irq_trigger.has_pending_irq(IrqType::Vring));
         // Check that the frame has been written successfully to the Rx descriptor chain.
-        th.rxq.check_used_elem(0, 3, frame.len() as u32);
+        th.rxq
+            .check_used_elem(0, 3, frame.len().try_into().unwrap());
         th.rxq.dtable[3].check_data(&frame[..100]);
         th.rxq.dtable[5].check_data(&frame[100..150]);
         th.rxq.dtable[11].check_data(&frame[150..]);
@@ -1225,11 +1227,13 @@ pub mod tests {
         assert_eq!(th.rxq.used.idx.get(), 2);
         assert!(&th.net().irq_trigger.has_pending_irq(IrqType::Vring));
         // Check that the 1st frame was written successfully to the 1st Rx descriptor chain.
-        th.rxq.check_used_elem(0, 0, frame_1.len() as u32);
+        th.rxq
+            .check_used_elem(0, 0, frame_1.len().try_into().unwrap());
         th.rxq.dtable[0].check_data(&frame_1);
         th.rxq.dtable[1].check_data(&[0; 500]);
         // Check that the 2nd frame was written successfully to the 2nd Rx descriptor chain.
-        th.rxq.check_used_elem(1, 2, frame_2.len() as u32);
+        th.rxq
+            .check_used_elem(1, 2, frame_2.len().try_into().unwrap());
         th.rxq.dtable[2].check_data(&frame_2);
         th.rxq.dtable[3].check_data(&[0; 500]);
     }
@@ -1821,7 +1825,8 @@ pub mod tests {
                 assert!(&th.net().irq_trigger.has_pending_irq(IrqType::Vring));
                 // make sure the data queue advanced
                 assert_eq!(th.rxq.used.idx.get(), 1);
-                th.rxq.check_used_elem(0, 0, frame.len() as u32);
+                th.rxq
+                    .check_used_elem(0, 0, frame.len().try_into().unwrap());
                 th.rxq.dtable[0].check_data(frame);
             }
         }
@@ -1930,7 +1935,8 @@ pub mod tests {
                 assert!(&th.net().irq_trigger.has_pending_irq(IrqType::Vring));
                 // make sure the data queue advanced
                 assert_eq!(th.rxq.used.idx.get(), 1);
-                th.rxq.check_used_elem(0, 0, frame.len() as u32);
+                th.rxq
+                    .check_used_elem(0, 0, frame.len().try_into().unwrap());
                 th.rxq.dtable[0].check_data(frame);
             }
         }
diff --git a/src/vmm/src/devices/virtio/net/test_utils.rs b/src/vmm/src/devices/virtio/net/test_utils.rs
@@ -516,7 +516,11 @@ pub mod test {
             self.add_desc_chain(
                 NetQueue::Rx,
                 0,
-                &[(0, expected_frame.len() as u32, VIRTQ_DESC_F_WRITE)],
+                &[(
+                    0,
+                    u32::try_from(expected_frame.len()).unwrap(),
+                    VIRTQ_DESC_F_WRITE,
+                )],
             );
             check_metric_after_block!(
                 METRICS.net.rx_packets_count,
@@ -527,7 +531,7 @@ pub mod test {
             assert_eq!(self.rxq.used.idx.get(), used_idx + 1);
             assert!(&self.net().irq_trigger.has_pending_irq(IrqType::Vring));
             self.rxq
-                .check_used_elem(used_idx, 0, expected_frame.len() as u32);
+                .check_used_elem(used_idx, 0, expected_frame.len().try_into().unwrap());
             self.rxq.dtable[0].check_data(expected_frame);
         }
 
diff --git a/src/vmm/src/devices/virtio/test_utils.rs b/src/vmm/src/devices/virtio/test_utils.rs
@@ -275,7 +275,8 @@ impl<'a> VirtQueue<'a> {
     }
 
     pub fn size(&self) -> u16 {
-        self.dtable.len() as u16
+        // Safe to unwrap because the size is specified as a u16 when the table is first created.
+        self.dtable.len().try_into().unwrap()
     }
 
     pub fn dtable_start(&self) -> GuestAddress {
diff --git a/src/vmm/src/devices/virtio/vsock/csm/connection.rs b/src/vmm/src/devices/virtio/vsock/csm/connection.rs
@@ -926,7 +926,7 @@ mod tests {
 
         fn init_data_pkt(&mut self, mut data: &[u8]) -> &VsockPacket {
             assert!(data.len() <= self.pkt.buf_size());
-            self.init_pkt(uapi::VSOCK_OP_RW, data.len() as u32);
+            self.init_pkt(uapi::VSOCK_OP_RW, u32::try_from(data.len()).unwrap());
 
             let len = data.len();
             self.pkt
diff --git a/src/vmm/src/devices/virtio/vsock/unix/muxer.rs b/src/vmm/src/devices/virtio/vsock/unix/muxer.rs
@@ -866,7 +866,7 @@ mod tests {
         ) -> &mut VsockPacket {
             assert!(data.len() <= self.pkt.buf_size());
             self.init_pkt(local_port, peer_port, uapi::VSOCK_OP_RW)
-                .set_len(data.len() as u32);
+                .set_len(u32::try_from(data.len()).unwrap());
 
             let data_len = data.len(); // store in tmp var to make borrow checker happy.
             self.pkt
diff --git a/src/vmm/src/dumbo/pdu/ipv4.rs b/src/vmm/src/dumbo/pdu/ipv4.rs
@@ -530,23 +530,19 @@ mod tests {
         let src = Ipv4Addr::new(10, 100, 11, 21);
         let dst = Ipv4Addr::new(192, 168, 121, 35);
 
-        let buf_len = buf.len();
+        let buf_len = u16::try_from(buf.len()).unwrap();
         // No IPv4 option support for now.
         let header_len = OPTIONS_OFFSET;
-        let payload_len = buf_len - usize::from(OPTIONS_OFFSET);
+        let payload_len = buf_len - u16::from(OPTIONS_OFFSET);
 
         {
             let mut p = IPv4Packet::write_header(buf.as_mut(), PROTOCOL_TCP, src, dst)
                 .unwrap()
-                .with_header_and_payload_len_unchecked(
-                    header_len,
-                    u16::try_from(payload_len).unwrap(),
-                    true,
-                );
+                .with_header_and_payload_len_unchecked(header_len, payload_len, true);
 
             assert_eq!(p.version_and_header_len(), (IPV4_VERSION, header_len));
             assert_eq!(p.dscp_and_ecn(), (0, 0));
-            assert_eq!(p.total_len() as usize, buf_len);
+            assert_eq!(p.total_len(), buf_len);
             assert_eq!(p.identification(), 0);
             assert_eq!(p.flags_and_fragment_offset(), (0, 0));
             assert_eq!(p.ttl(), DEFAULT_TTL);
@@ -604,16 +600,11 @@ mod tests {
         look_for_error(buf.as_ref(), Error::InvalidTotalLen);
 
         // Total len not matching slice length.
-        p(buf.as_mut()).set_total_len(buf_len as u16 - 1);
+        p(buf.as_mut()).set_total_len(buf_len - 1);
         look_for_error(buf.as_ref(), Error::SliceExactLen);
 
         // The original packet header should contain a valid checksum.
-        assert_eq!(
-            p(buf.as_mut())
-                .set_total_len(buf_len as u16)
-                .compute_checksum(),
-            0
-        );
+        assert_eq!(p(buf.as_mut()).set_total_len(buf_len).compute_checksum(), 0);
 
         // Let's make it invalid.
         let checksum = p(buf.as_mut()).header_checksum();
diff --git a/src/vmm/src/dumbo/pdu/udp.rs b/src/vmm/src/dumbo/pdu/udp.rs
@@ -242,7 +242,7 @@ mod tests {
         let payload_length = total_len - UDP_HEADER_SIZE;
         assert_eq!(p.payload().len(), payload_length);
 
-        let payload: Vec<u8> = (0..(payload_length as u8)).collect();
+        let payload: Vec<u8> = (0..u8::try_from(payload_length).unwrap()).collect();
         p.payload_mut().copy_from_slice(&payload);
         assert_eq!(*p.payload(), payload[..]);
     }
diff --git a/src/vmm/src/dumbo/tcp/connection.rs b/src/vmm/src/dumbo/tcp/connection.rs
@@ -1778,7 +1778,7 @@ pub(crate) mod tests {
             check_control_segment(&s, 0, TcpFlags::FIN | TcpFlags::ACK);
             assert_eq!(
                 s.sequence_number(),
-                conn_isn.wrapping_add(1 + bytes_sent_by_c as u32)
+                conn_isn.wrapping_add(1 + u32::try_from(bytes_sent_by_c).unwrap())
             );
         }
 
diff --git a/src/vmm/src/dumbo/tcp/endpoint.rs b/src/vmm/src/dumbo/tcp/endpoint.rs
@@ -444,7 +444,7 @@ mod tests {
 
         // 1 for the SYN.
         let mut remote_first_not_sent =
-            remote_isn.wrapping_add(1 + incomplete_request.len() as u32);
+            remote_isn.wrapping_add(1 + u32::try_from(incomplete_request.len()).unwrap());
 
         // The endpoint should write an ACK at this point.
         {
@@ -470,7 +470,7 @@ mod tests {
         }
 
         remote_first_not_sent =
-            remote_first_not_sent.wrapping_add(rest_of_the_request.len() as u32);
+            remote_first_not_sent.wrapping_add(rest_of_the_request.len().try_into().unwrap());
 
         let mut endpoint_first_not_sent;
 
@@ -521,7 +521,8 @@ mod tests {
                 endpoint.receive_segment(&data, mock_callback);
             }
 
-            remote_first_not_sent = remote_first_not_sent.wrapping_add(request.len() as u32);
+            remote_first_not_sent =
+                remote_first_not_sent.wrapping_add(request.len().try_into().unwrap());
 
             // Check response.
             {
@@ -558,7 +559,7 @@ mod tests {
 
         {
             // Hack: have to artificially increase t.mss to create this segment which is 2k+.
-            t.mss = RCV_BUF_MAX_SIZE as u16;
+            t.mss = RCV_BUF_MAX_SIZE.try_into().unwrap();
             let mut data = t.write_data(write_buf.as_mut(), request_to_fill.as_ref());
 
             data.set_flags_after_ns(TcpFlags::ACK);
diff --git a/src/vmm/src/io_uring/mod.rs b/src/vmm/src/io_uring/mod.rs
@@ -487,7 +487,7 @@ mod tests {
         for i in 0..FILE_LEN {
             write_mem_region
                 .as_volatile_slice()
-                .write_obj((i % u32::from(u8::MAX)) as u8, i as usize)
+                .write_obj(u8::try_from(i % u32::from(u8::MAX)).unwrap(), i as usize)
                 .unwrap();
         }
 

Original file line number	Diff line number	Diff line change
`@@ -917,7 +917,7 @@ mod tests {`
`917`	`917`	`assert_eq!(rc, 0);`
`918`	`918`	`}`
`919`	`919`	`let bpf_prog = sock_fprog {`
`920`		`- len: bpf_filter.len() as u16,`
	`920`	`+ len: u16::try_from(bpf_filter.len()).unwrap(),`
`921`	`921`	`filter: bpf_filter.as_ptr(),`
`922`	`922`	`};`
`923`	`923`	`let bpf_prog_ptr = &bpf_prog as *const sock_fprog;`
Original file line number	Diff line number	Diff line change
`@@ -412,7 +412,7 @@ mod tests {`
`412`	`412`
`413`	`413`	`// Test buffer full.`
`414`	`414`	`for i in 0..BUF_SIZE {`
`415`		`- i8042.push_byte(i as u8).unwrap();`
	`415`	`+ i8042.push_byte(i.try_into().unwrap()).unwrap();`
`416`	`416`	`assert_eq!(i8042.buf_len(), i + 1);`
`417`	`417`	`}`
`418`	`418`	`assert_eq!(`
Original file line number	Diff line number	Diff line change
`@@ -275,7 +275,8 @@ impl<'a> VirtQueue<'a> {`
`275`	`275`	`}`
`276`	`276`
`277`	`277`	`pub fn size(&self) -> u16 {`
`278`		`- self.dtable.len() as u16`
	`278`	`+ // Safe to unwrap because the size is specified as a u16 when the table is first created.`
	`279`	`+ self.dtable.len().try_into().unwrap()`
`279`	`280`	`}`
`280`	`281`
`281`	`282`	`pub fn dtable_start(&self) -> GuestAddress {`