Merge pull request #179 from immunant/kkysen/switch-to-standard-SAFETY-comments

Change `// Safe because` comments to proper `// SAFETY:` comments `clippy` recognizes

Author: qwandor

src/device/console.rs

@@ -142,7 +142,7 @@ impl<H: Hal, T: Transport> VirtIOConsole<H, T> {
     /// receive request or some data already received and not yet returned.
     fn poll_retrieve(&mut self) -> Result<()> {
         if self.receive_token.is_none() && self.cursor == self.pending_len {
-            // Safe because the buffer lasts at least as long as the queue, and there are no other
+            // SAFETY: The buffer lasts at least as long as the queue, and there are no other
             // outstanding requests using the buffer.
             self.receive_token = Some(unsafe {
                 self.receiveq
@@ -174,7 +174,7 @@ impl<H: Hal, T: Transport> VirtIOConsole<H, T> {
         let mut flag = false;
         if let Some(receive_token) = self.receive_token {
             if self.receive_token == self.receiveq.peek_used() {
-                // Safe because we are passing the same buffer as we passed to `VirtQueue::add` in
+                // SAFETY: We are passing the same buffer as we passed to `VirtQueue::add` in
                 // `poll_retrieve` and it is still valid.
                 let len = unsafe {
                     self.receiveq.pop_used(

src/device/input.rs

@@ -43,7 +43,7 @@ impl<H: Hal, T: Transport> VirtIOInput<H, T> {
             negotiated_features.contains(Feature::RING_EVENT_IDX),
         )?;
         for (i, event) in event_buf.as_mut().iter_mut().enumerate() {
-            // Safe because the buffer lasts as long as the queue.
+            // SAFETY: The buffer lasts as long as the queue.
             let token = unsafe { event_queue.add(&[], &mut [event.as_mut_bytes()])? };
             assert_eq!(token, i as u16);
         }
@@ -70,16 +70,15 @@ impl<H: Hal, T: Transport> VirtIOInput<H, T> {
     pub fn pop_pending_event(&mut self) -> Option<InputEvent> {
         if let Some(token) = self.event_queue.peek_used() {
             let event = &mut self.event_buf[token as usize];
-            // Safe because we are passing the same buffer as we passed to `VirtQueue::add` and it
-            // is still valid.
+            // SAFETY: We are passing the same buffer as we passed to `VirtQueue::add` and it is still valid.
             unsafe {
                 self.event_queue
                     .pop_used(token, &[], &mut [event.as_mut_bytes()])
                     .ok()?;
             }
             let event_saved = *event;
             // requeue
-            // Safe because buffer lasts as long as the queue.
+            // SAFETY: The buffer lasts as long as the queue.
             if let Ok(new_token) = unsafe { self.event_queue.add(&[], &mut [event.as_mut_bytes()]) }
             {
                 // This only works because nothing happen between `pop_used` and `add` that affects

src/device/net/dev.rs

@@ -31,7 +31,7 @@ impl<H: Hal, T: Transport, const QUEUE_SIZE: usize> VirtIONet<H, T, QUEUE_SIZE>
         let mut rx_buffers = [NONE_BUF; QUEUE_SIZE];
         for (i, rx_buf_place) in rx_buffers.iter_mut().enumerate() {
             let mut rx_buf = RxBuffer::new(i, buf_len);
-            // Safe because the buffer lives as long as the queue.
+            // SAFETY: The buffer lives as long as the queue.
             let token = unsafe { inner.receive_begin(rx_buf.as_bytes_mut())? };
             assert_eq!(token, i as u16);
             *rx_buf_place = Some(rx_buf);
@@ -84,7 +84,7 @@ impl<H: Hal, T: Transport, const QUEUE_SIZE: usize> VirtIONet<H, T, QUEUE_SIZE>
                 return Err(Error::WrongToken);
             }
 
-            // Safe because `token` == `rx_buf.idx`, we are passing the same
+            // SAFETY: `token` == `rx_buf.idx`, so we are passing the same
             // buffer as we passed to `VirtQueue::add` and it is still valid.
             let (_hdr_len, pkt_len) =
                 unsafe { self.inner.receive_complete(token, rx_buf.as_bytes_mut())? };
@@ -99,8 +99,8 @@ impl<H: Hal, T: Transport, const QUEUE_SIZE: usize> VirtIONet<H, T, QUEUE_SIZE>
     ///
     /// It will add the buffer back to the NIC queue.
     pub fn recycle_rx_buffer(&mut self, mut rx_buf: RxBuffer) -> Result {
-        // Safe because we take the ownership of `rx_buf` back to `rx_buffers`,
-        // it lives as long as the queue.
+        // SAFETY: We take the ownership of `rx_buf` back to `rx_buffers`,
+        // so it lives as long as the queue.
         let new_token = unsafe { self.inner.receive_begin(rx_buf.as_bytes_mut()) }?;
         // `rx_buffers[new_token]` is expected to be `None` since it was taken
         // away at `Self::receive()` and has not been added back.

src/hal.rs

@@ -62,8 +62,8 @@ impl<H: Hal> Dma<H> {
 
 impl<H: Hal> Drop for Dma<H> {
     fn drop(&mut self) {
-        // Safe because the memory was previously allocated by `dma_alloc` in `Dma::new`, not yet
-        // deallocated, and we are passing the values from then.
+        // SAFETY: The memory was previously allocated by `dma_alloc` in `Dma::new`,
+        // not yet deallocated, and we are passing the values from then.
         let err = unsafe { H::dma_dealloc(self.paddr, self.vaddr, self.pages) };
         assert_eq!(err, 0, "failed to deallocate DMA");
     }

src/queue.rs

@@ -114,8 +114,8 @@ impl<H: Hal, const SIZE: usize> VirtQueue<H, SIZE> {
         // Link descriptors together.
         for i in 0..(size - 1) {
             desc_shadow[i as usize].next = i + 1;
-            // Safe because `desc` is properly aligned, dereferenceable, initialised, and the device
-            // won't access the descriptors for the duration of this unsafe block.
+            // SAFETY: `desc` is properly aligned, dereferenceable, initialised,
+            // and the device won't access the descriptors for the duration of this unsafe block.
             unsafe {
                 (*desc.as_ptr())[i as usize].next = i + 1;
             }
@@ -185,7 +185,7 @@ impl<H: Hal, const SIZE: usize> VirtQueue<H, SIZE> {
         let head = self.add_direct(inputs, outputs);
 
         let avail_slot = self.avail_idx & (SIZE as u16 - 1);
-        // Safe because self.avail is properly aligned, dereferenceable and initialised.
+        // SAFETY: `self.avail` is properly aligned, dereferenceable and initialised.
         unsafe {
             (*self.avail.as_ptr()).ring[avail_slot as usize] = head;
         }
@@ -196,7 +196,7 @@ impl<H: Hal, const SIZE: usize> VirtQueue<H, SIZE> {
 
         // increase head of avail ring
         self.avail_idx = self.avail_idx.wrapping_add(1);
-        // Safe because self.avail is properly aligned, dereferenceable and initialised.
+        // SAFETY: `self.avail` is properly aligned, dereferenceable and initialised.
         unsafe {
             (*self.avail.as_ptr())
                 .idx
@@ -220,7 +220,7 @@ impl<H: Hal, const SIZE: usize> VirtQueue<H, SIZE> {
 
             // Write to desc_shadow then copy.
             let desc = &mut self.desc_shadow[usize::from(self.free_head)];
-            // Safe because our caller promises that the buffers live at least until `pop_used`
+            // SAFETY: Our caller promises that the buffers live at least until `pop_used`
             // returns them.
             unsafe {
                 desc.set_buf::<H>(buffer, direction, DescFlags::NEXT);
@@ -255,7 +255,7 @@ impl<H: Hal, const SIZE: usize> VirtQueue<H, SIZE> {
             <[Descriptor]>::new_box_zeroed_with_elems(inputs.len() + outputs.len()).unwrap();
         for (i, (buffer, direction)) in InputOutputIter::new(inputs, outputs).enumerate() {
             let desc = &mut indirect_list[i];
-            // Safe because our caller promises that the buffers live at least until `pop_used`
+            // SAFETY: Our caller promises that the buffers live at least until `pop_used`
             // returns them.
             unsafe {
                 desc.set_buf::<H>(buffer, direction, DescFlags::NEXT);
@@ -303,7 +303,7 @@ impl<H: Hal, const SIZE: usize> VirtQueue<H, SIZE> {
         outputs: &'a mut [&'a mut [u8]],
         transport: &mut impl Transport,
     ) -> Result<u32> {
-        // Safe because we don't return until the same token has been popped, so the buffers remain
+        // SAFETY: We don't return until the same token has been popped, so the buffers remain
         // valid and are not otherwise accessed until then.
         let token = unsafe { self.add(inputs, outputs) }?;
 
@@ -317,8 +317,7 @@ impl<H: Hal, const SIZE: usize> VirtQueue<H, SIZE> {
             spin_loop();
         }
 
-        // Safe because these are the same buffers as we passed to `add` above and they are still
-        // valid.
+        // SAFETY: These are the same buffers as we passed to `add` above and they are still valid.
         unsafe { self.pop_used(token, inputs, outputs) }
     }
 
@@ -328,8 +327,8 @@ impl<H: Hal, const SIZE: usize> VirtQueue<H, SIZE> {
     pub fn set_dev_notify(&mut self, enable: bool) {
         let avail_ring_flags = if enable { 0x0000 } else { 0x0001 };
         if !self.event_idx {
-            // Safe because self.avail points to a valid, aligned, initialised, dereferenceable, readable
-            // instance of AvailRing.
+            // SAFETY: `self.avail` points to a valid, aligned, initialised, dereferenceable, readable
+            // instance of `AvailRing`.
             unsafe {
                 (*self.avail.as_ptr())
                     .flags
@@ -344,13 +343,13 @@ impl<H: Hal, const SIZE: usize> VirtQueue<H, SIZE> {
     /// This will be false if the device has supressed notifications.
     pub fn should_notify(&self) -> bool {
         if self.event_idx {
-            // Safe because self.used points to a valid, aligned, initialised, dereferenceable, readable
-            // instance of UsedRing.
+            // SAFETY: `self.used` points to a valid, aligned, initialised, dereferenceable, readable
+            // instance of `UsedRing`.
             let avail_event = unsafe { (*self.used.as_ptr()).avail_event.load(Ordering::Acquire) };
             self.avail_idx >= avail_event.wrapping_add(1)
         } else {
-            // Safe because self.used points to a valid, aligned, initialised, dereferenceable, readable
-            // instance of UsedRing.
+            // SAFETY: `self.used` points to a valid, aligned, initialised, dereferenceable, readable
+            // instance of `UsedRing`.
             unsafe { (*self.used.as_ptr()).flags.load(Ordering::Acquire) & 0x0001 == 0 }
         }
     }
@@ -359,7 +358,7 @@ impl<H: Hal, const SIZE: usize> VirtQueue<H, SIZE> {
     /// the device.
     fn write_desc(&mut self, index: u16) {
         let index = usize::from(index);
-        // Safe because self.desc is properly aligned, dereferenceable and initialised, and nothing
+        // SAFETY: `self.desc` is properly aligned, dereferenceable and initialised, and nothing
         // else reads or writes the descriptor during this block.
         unsafe {
             (*self.desc.as_ptr())[index] = self.desc_shadow[index].clone();
@@ -368,8 +367,8 @@ impl<H: Hal, const SIZE: usize> VirtQueue<H, SIZE> {
 
     /// Returns whether there is a used element that can be popped.
     pub fn can_pop(&self) -> bool {
-        // Safe because self.used points to a valid, aligned, initialised, dereferenceable, readable
-        // instance of UsedRing.
+        // SAFETY: `self.used` points to a valid, aligned, initialised, dereferenceable, readable
+        // instance of `UsedRing`.
         self.last_used_idx != unsafe { (*self.used.as_ptr()).idx.load(Ordering::Acquire) }
     }
 
@@ -378,8 +377,8 @@ impl<H: Hal, const SIZE: usize> VirtQueue<H, SIZE> {
     pub fn peek_used(&self) -> Option<u16> {
         if self.can_pop() {
             let last_used_slot = self.last_used_idx & (SIZE as u16 - 1);
-            // Safe because self.used points to a valid, aligned, initialised, dereferenceable,
-            // readable instance of UsedRing.
+            // SAFETY: `self.used` points to a valid, aligned, initialised, dereferenceable,
+            // readable instance of `UsedRing`.
             Some(unsafe { (*self.used.as_ptr()).ring[last_used_slot as usize].id as u16 })
         } else {
             None
@@ -513,8 +512,8 @@ impl<H: Hal, const SIZE: usize> VirtQueue<H, SIZE> {
         let last_used_slot = self.last_used_idx & (SIZE as u16 - 1);
         let index;
         let len;
-        // Safe because self.used points to a valid, aligned, initialised, dereferenceable, readable
-        // instance of UsedRing.
+        // SAFETY: `self.used` points to a valid, aligned, initialised, dereferenceable, readable
+        // instance of `UsedRing`.
         unsafe {
             index = (*self.used.as_ptr()).ring[last_used_slot as usize].id as u16;
             len = (*self.used.as_ptr()).ring[last_used_slot as usize].len;
@@ -525,7 +524,7 @@ impl<H: Hal, const SIZE: usize> VirtQueue<H, SIZE> {
             return Err(Error::WrongToken);
         }
 
-        // Safe because the caller ensures the buffers are valid and match the descriptor.
+        // SAFETY: The caller ensures the buffers are valid and match the descriptor.
         unsafe {
             self.recycle_descriptors(index, inputs, outputs);
         }
@@ -718,7 +717,7 @@ impl Descriptor {
         direction: BufferDirection,
         extra_flags: DescFlags,
     ) {
-        // Safe because our caller promises that the buffer is valid.
+        // SAFETY: Our caller promises that the buffer is valid.
         unsafe {
             self.addr = H::share(buf, direction) as u64;
         }

src/queue/owning.rs

@@ -146,7 +146,7 @@ impl<H: Hal, const SIZE: usize, const BUFFER_SIZE: usize> Drop
 {
     fn drop(&mut self) {
         for buffer in self.buffers {
-            // Safe because we obtained the buffer pointer from Box::into_raw, and it won't be used
+            // SAFETY: We obtained the buffer pointer from `Box::into_raw`, and it won't be used
             // anywhere else after the queue is destroyed.
             unsafe { drop(Box::from_raw(buffer.as_ptr())) };
         }

src/transport/mmio.rs

@@ -304,7 +304,7 @@ impl MmioTransport {
 
     /// Gets the vendor ID.
     pub fn vendor_id(&self) -> u32 {
-        // Safe because self.header points to a valid VirtIO MMIO region.
+        // SAFETY: `self.header` points to a valid VirtIO MMIO region.
         unsafe { volread!(self.header, vendor_id) }
     }
 }
@@ -318,13 +318,13 @@ unsafe impl Sync for MmioTransport {}
 
 impl Transport for MmioTransport {
     fn device_type(&self) -> DeviceType {
-        // Safe because self.header points to a valid VirtIO MMIO region.
+        // SAFETY: `self.header` points to a valid VirtIO MMIO region.
         let device_id = unsafe { volread!(self.header, device_id) };
         device_id.into()
     }
 
     fn read_device_features(&mut self) -> u64 {
-        // Safe because self.header points to a valid VirtIO MMIO region.
+        // SAFETY: `self.header` points to a valid VirtIO MMIO region.
         unsafe {
             volwrite!(self.header, device_features_sel, 0); // device features [0, 32)
             let mut device_features_bits = volread!(self.header, device_features).into();
@@ -335,7 +335,7 @@ impl Transport for MmioTransport {
     }
 
     fn write_driver_features(&mut self, driver_features: u64) {
-        // Safe because self.header points to a valid VirtIO MMIO region.
+        // SAFETY: `self.header` points to a valid VirtIO MMIO region.
         unsafe {
             volwrite!(self.header, driver_features_sel, 0); // driver features [0, 32)
             volwrite!(self.header, driver_features, driver_features as u32);
@@ -345,27 +345,27 @@ impl Transport for MmioTransport {
     }
 
     fn max_queue_size(&mut self, queue: u16) -> u32 {
-        // Safe because self.header points to a valid VirtIO MMIO region.
+        // SAFETY: `self.header` points to a valid VirtIO MMIO region.
         unsafe {
             volwrite!(self.header, queue_sel, queue.into());
             volread!(self.header, queue_num_max)
         }
     }
 
     fn notify(&mut self, queue: u16) {
-        // Safe because self.header points to a valid VirtIO MMIO region.
+        // SAFETY: `self.header` points to a valid VirtIO MMIO region.
         unsafe {
             volwrite!(self.header, queue_notify, queue.into());
         }
     }
 
     fn get_status(&self) -> DeviceStatus {
-        // Safe because self.header points to a valid VirtIO MMIO region.
+        // SAFETY: `self.header` points to a valid VirtIO MMIO region.
         unsafe { volread!(self.header, status) }
     }
 
     fn set_status(&mut self, status: DeviceStatus) {
-        // Safe because self.header points to a valid VirtIO MMIO region.
+        // SAFETY: `self.header` points to a valid VirtIO MMIO region.
         unsafe {
             volwrite!(self.header, status, status);
         }
@@ -374,7 +374,7 @@ impl Transport for MmioTransport {
     fn set_guest_page_size(&mut self, guest_page_size: u32) {
         match self.version {
             MmioVersion::Legacy => {
-                // Safe because self.header points to a valid VirtIO MMIO region.
+                // SAFETY: `self.header` points to a valid VirtIO MMIO region.
                 unsafe {
                     volwrite!(self.header, legacy_guest_page_size, guest_page_size);
                 }
@@ -416,7 +416,7 @@ impl Transport for MmioTransport {
                 let align = PAGE_SIZE as u32;
                 let pfn = (descriptors / PAGE_SIZE) as u32;
                 assert_eq!(pfn as usize * PAGE_SIZE, descriptors);
-                // Safe because self.header points to a valid VirtIO MMIO region.
+                // SAFETY: `self.header` points to a valid VirtIO MMIO region.
                 unsafe {
                     volwrite!(self.header, queue_sel, queue.into());
                     volwrite!(self.header, queue_num, size);
@@ -425,7 +425,7 @@ impl Transport for MmioTransport {
                 }
             }
             MmioVersion::Modern => {
-                // Safe because self.header points to a valid VirtIO MMIO region.
+                // SAFETY: `self.header` points to a valid VirtIO MMIO region.
                 unsafe {
                     volwrite!(self.header, queue_sel, queue.into());
                     volwrite!(self.header, queue_num, size);
@@ -444,7 +444,7 @@ impl Transport for MmioTransport {
     fn queue_unset(&mut self, queue: u16) {
         match self.version {
             MmioVersion::Legacy => {
-                // Safe because self.header points to a valid VirtIO MMIO region.
+                // SAFETY: `self.header` points to a valid VirtIO MMIO region.
                 unsafe {
                     volwrite!(self.header, queue_sel, queue.into());
                     volwrite!(self.header, queue_num, 0);
@@ -453,7 +453,7 @@ impl Transport for MmioTransport {
                 }
             }
             MmioVersion::Modern => {
-                // Safe because self.header points to a valid VirtIO MMIO region.
+                // SAFETY: `self.header` points to a valid VirtIO MMIO region.
                 unsafe {
                     volwrite!(self.header, queue_sel, queue.into());
 
@@ -474,7 +474,7 @@ impl Transport for MmioTransport {
     }
 
     fn queue_used(&mut self, queue: u16) -> bool {
-        // Safe because self.header points to a valid VirtIO MMIO region.
+        // SAFETY: `self.header` points to a valid VirtIO MMIO region.
         unsafe {
             volwrite!(self.header, queue_sel, queue.into());
             match self.version {
@@ -485,7 +485,7 @@ impl Transport for MmioTransport {
     }
 
     fn ack_interrupt(&mut self) -> bool {
-        // Safe because self.header points to a valid VirtIO MMIO region.
+        // SAFETY: `self.header` points to a valid VirtIO MMIO region.
         unsafe {
             let interrupt = volread!(self.header, interrupt_status);
             if interrupt != 0 {