Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mmap_xen: Update to bitflags 2.4.0 #255

Merged
merged 1 commit into from
Oct 3, 2023
Merged

Conversation

vireshk
Copy link
Contributor

@vireshk vireshk commented Oct 3, 2023

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:

  • All commits in this PR are signed (with git commit -s), and the commit
    message has max 60 characters for the summary and max 75 characters for each
    description line.
  • All added/changed functionality has a corresponding unit/integration
    test.
  • All added/changed public-facing functionality has entries in the "Upcoming
    Release" section of CHANGELOG.md (if no such section exists, please create one).
  • Any newly added unsafe code is properly documented.

@stefano-garzarella
Copy link
Member

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.

@vireshk vireshk changed the title mmap_xen: Bitflags should have non-zero fields mmap_xen: Update to bitflags 2.4.0 Oct 3, 2023
@stefano-garzarella
Copy link
Member

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
Copy link
Contributor Author

vireshk commented Oct 3, 2023

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
Copy link
Member

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

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

src/mmap_xen.rs Outdated Show resolved Hide resolved
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 by switching to a newer version of bitflags.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
@roypat roypat merged commit 961a017 into rust-vmm:main Oct 3, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants