rust-vmm / vm-virtio

virtio implementation
Apache License 2.0
366 stars 87 forks source link

Update documentation of avail_idx & used_idx #174

Closed andreeaflorescu closed 2 years ago

andreeaflorescu commented 2 years ago

Summary of the PR

The avail_idx and used_idx methods panic if the ordering is not the right one. As checking the ordering incurs a performance degradation, the VMM will be responsible for calling these functions only with the right ordering. We're now making this panic explicit by adding it to the public documentation.

Requirements

Before submitting your PR, please make sure you addressed the following requirements:

jiangliu commented 2 years ago

How abouting using "debug_assert()" instead of runtime checking?

andreeaflorescu commented 2 years ago

How abouting using "debug_assert()" instead of runtime checking?

@jiangliu this will still cause a panic. In case we don't want to fix this by checking the operation, than we can just leave it as is and add a comment to the function specifying that the function panics in case it's called with unsupported ordering. WDYT?

jiangliu commented 2 years ago

How abouting using "debug_assert()" instead of runtime checking?

@jiangliu this will still cause a panic. In case we don't want to fix this by checking the operation, than we can just leave it as is and add a comment to the function specifying that the function panics in case it's called with unsupported ordering. WDYT?

I feel it's a type of program error instead of a runtime error, so not sure about the runtime performance impact.