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

Code before:
        // 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.
        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.
        let desc_index: u16 = mem.read_obj(desc_index_address).unwrap();

        DescriptorChain::checked_new(mem, self.desc_table, self.actual_size(), desc_index).map(
            |dc| {
                self.next_avail += Wrapping(1);
                dc
            },
        )
    }

    /// Undo the effects of the last `self.pop()` call.
    /// The caller can use this, if it was unable to consume the last popped descriptor chain.
    pub fn undo_pop(&mut self) {
        self.next_avail -= Wrapping(1);
    }

    /// Puts an available descriptor head into the used ring for use by the guest.
    pub fn add_used<M: GuestMemory>(
        &mut self,
        mem: &M,
        desc_index: u16,
        len: u32,
    ) -> Result<(), QueueError> {
        debug_assert!(self.is_layout_valid(mem));

        let next_used = self.next_used.0 % self.actual_size();
        let used_element = UsedElement {
            id: u32::from(desc_index),
            len,
        };
        self.write_used_ring(mem, next_used, used_element)?;


Review comment (suggestion):
There's not need to remove the comments of this section. Even though you use `size_of` now to do the calculations. but the explanation here is still valid

Code after:
        // 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.
        let desc_index: u16 = mem.read_obj(desc_index_address).unwrap();

        DescriptorChain::checked_new(mem, self.desc_table, self.actual_size(), desc_index).map(
            |dc| {
                self.next_avail += Wrapping(1);
                dc
            },
        )
    }

    /// Undo the effects of the last `self.pop()` call.
    /// The caller can use this, if it was unable to consume the last popped descriptor chain.
    pub fn undo_pop(&mut self) {
        self.next_avail -= Wrapping(1);
    }

    /// Puts an available descriptor head into the used ring for use by the guest.
    pub fn add_used<M: GuestMemory>(
        &mut self,
        mem: &M,
        desc_index: u16,
        len: u32,
    ) -> Result<(), QueueError> {