rust-vmm / vm-memory

Virtual machine's guest memory crate
Apache License 2.0
299 stars 97 forks source link

mmap_xen: Update to bitflags 2.4.0 #255

Closed vireshk closed 10 months ago

vireshk commented 10 months ago

Clippy gives this warning currently:

error: &-masking with zero --> src/mmap_xen.rs:433:1 433 / bitflags! { 434 /// Flags for the Xen mmap message. 435 pub struct MmapXenFlags: u32 { 436 /// Standard Unix memory mapping. ... 446 } 447 } _^
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#bad_bit_mask
= note: `#[deny(clippy::bad_bit_mask)]` on by default
= note: this error originates in the macro `__impl_bitflags` which comes from the expansion of the macro `bitflags` (in Nightly builds, run with -Z macro-backtrace for more info)

Fix it.

Summary of the PR

Please summarize here why the changes in this PR are needed.

Requirements

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

stefano-garzarella commented 10 months ago

I don't know if it's a real bug or not, but if you want to keep the previous values, you can switch to bitflags 2.* and you shouldn't see the warning anymore.

stefano-garzarella commented 10 months ago

Not for this PR, but I was looking at the code, and I'm a bit suspicious of:

    /// Is standard Unix memory.
    pub fn is_unix(&self) -> bool {
        self.bits() == Self::UNIX.bits()
    }

Can a Unix memory have NO_ADVANCE_MAP flag set? If it is the case is_unix() will return false, so we may need a mask there.

vireshk commented 10 months ago

Not for this PR, but I was looking at the code, and I'm a bit suspicious of:

    /// Is standard Unix memory.
    pub fn is_unix(&self) -> bool {
        self.bits() == Self::UNIX.bits()
    }

Can a Unix memory have NO_ADVANCE_MAP flag set? If it is the case is_unix() will return false, so we may need a mask there.

Unix can't have NO_ADVANCE_MAP flag. Only Grant mapping can have that.

stefano-garzarella commented 10 months ago

Unix can't have NO_ADVANCE_MAP flag. Only Grant mapping can have that.

Oh I see, I was confused by the negative flag!