rust-vmm / vhost

Apache License 2.0
126 stars 64 forks source link

vhost-user-backend: take reference to `vring` once #214

Closed stefano-garzarella closed 7 months ago

stefano-garzarella commented 7 months ago

vhost-user-backend: take reference to vring once

In many places we access the VhostUserHandler::vrings array many times and at the beginning of each function we check the index to avoid OOB.

It happened that we forgot about this check (see #211), so let's avoid this error-prone pattern by taking the reference to the vring using Vec::get(), and avoiding multiple indexing.

Suggested-by: Erik Schilling erik.schilling@linaro.org Signed-off-by: Stefano Garzarella sgarzare@redhat.com

stefano-garzarella commented 7 months ago

Depends on #212

Ablu commented 7 months ago

Thanks :)

stefano-garzarella commented 7 months ago

Thanks :)

Thank you for the inspiration ;-)

I don't know why, but this change reduces the coverage, mmm. I'll update the score.

Ablu commented 7 months ago

I don't know why, but this change reduces the coverage, mmm. I'll update the score.

Probably the error case never got exercised. But I would argue that it was an issue with the existing tests and the previous coverage was just "wrong" :). So adjusting the coverage sounds fine to me!

stefano-garzarella commented 7 months ago

212 is now merged, so I rebased on main and marged ready