Repository: firecracker-microvm/firecracker
File: src/vmm/src/devices/virtio/queue.rs (Rust)

Code before:
            return self.pop(mem);
        }

        if self.try_enable_notification(mem) {
            return None;
        }

        self.do_pop_unchecked(mem)
    }

    /// Pop the first available descriptor chain from the avail ring.
    ///
    /// # Important
    /// This is an internal method that ASSUMES THAT THERE ARE AVAILABLE DESCRIPTORS. Otherwise it
    /// will retrieve a descriptor that contains garbage data (obsolete/empty).
    fn do_pop_unchecked<'b, M: GuestMemory>(
        &mut self,
        mem: &'b M,
    ) -> Option<DescriptorChain<'b, M>> {
        self.get_desc_chain(self.next_avail.0, mem).map(|dc| {
            self.next_avail += Wrapping(1);
            dc
        })
    }

    /// Get descriptor chain from the avail ring at the specified index.
    pub fn get_desc_chain<'b, M: GuestMemory>(
        &self,
        avail_idx: u16,
        mem: &'b M,
    ) -> Option<DescriptorChain<'b, M>> {
        // This fence ensures all subsequent reads see the updated driver writes.
        fence(Ordering::Acquire);

        // Avail ring has layout:
        // struct AvailRing {
        //     flags: u16,
        //     idx: u16,
        //     ring: [u16; <queue size>],
        //     used_event: u16,
        // }
        // We calculate offset into `ring` field.
        let desc_index_offset = std::mem::size_of::<u16>()
            + std::mem::size_of::<u16>()
            + std::mem::size_of::<u16>() * usize::from(avail_idx % self.actual_size());
        let desc_index_address = self
            .avail_ring
            .unchecked_add(usize_to_u64(desc_index_offset));

        // SAFETY:
        // `desc_index_address` param is bounded by size of the queue as `avail_idx` is

Review comment (suggestion):
```suggestion
        self.get_desc_chain(self.next_avail.0, mem).inspect(|_| {
            self.next_avail += Wrapping(1);
        })
```

Code after:
        &mut self,
        mem: &'b M,
    ) -> Option<DescriptorChain<'b, M>> {
        if !self.uses_notif_suppression {
            return self.pop(mem);
        }

        if self.try_enable_notification(mem) {
            return None;
        }

        self.do_pop_unchecked(mem)
    }

    /// Pop the first available descriptor chain from the avail ring.
    ///
    /// # Important
    /// This is an internal method that ASSUMES THAT THERE ARE AVAILABLE DESCRIPTORS. Otherwise it
    /// will retrieve a descriptor that contains garbage data (obsolete/empty).
    fn do_pop_unchecked<'b, M: GuestMemory>(
        &mut self,
        mem: &'b M,
    ) -> Option<DescriptorChain<'b, M>> {
        // This fence ensures all subsequent reads see the updated driver writes.
        fence(Ordering::Acquire);

        // We'll need to find the first available descriptor, that we haven't yet popped.
        // In a naive notation, that would be:
        // `descriptor_table[avail_ring[next_avail]]`.
        //
        // Avail ring has layout:
        // struct AvailRing {
        //     flags: u16,
        //     idx: u16,
        //     ring: [u16; <queue size>],
        //     used_event: u16,
        // }
        // We calculate offset into `ring` field.
        // We use `self.next_avail` to store the position, of the next available descriptor
        // index in the `ring` field. Because `self.next_avail` is only incremented, the actual
        // index into `AvailRing` is `self.next_avail % self.actual_size()`.
        let desc_index_offset = std::mem::size_of::<u16>()
            + std::mem::size_of::<u16>()
            + std::mem::size_of::<u16>() * usize::from(self.next_avail.0 % self.actual_size());
        let desc_index_address = self
            .avail_ring
            .unchecked_add(usize_to_u64(desc_index_offset));

        // `self.is_valid()` already performed all the bound checks on the descriptor table
        // and virtq rings, so it's safe to unwrap guest memory reads and to use unchecked
        // offsets.