Reviewed-by: Manos Pitsidianakis <manos.pitsidiana...@linaro.org> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> --- docs/devel/rust.rst | 4 -- rust/hw/timer/hpet/src/fw_cfg.rs | 6 +- rust/qemu-api/src/zeroable.rs | 104 +++++-------------------------- 3 files changed, 18 insertions(+), 96 deletions(-)
diff --git a/docs/devel/rust.rst b/docs/devel/rust.rst index 8167ff49aa9..13a002cfe69 100644 --- a/docs/devel/rust.rst +++ b/docs/devel/rust.rst @@ -83,10 +83,6 @@ are missing: * "Return position ``impl Trait`` in Traits" (1.75.0, blocker for including the pinned-init create). -* ``MaybeUninit::zeroed()`` as a ``const`` function (1.75.0). QEMU's - ``Zeroable`` trait can be implemented without ``MaybeUninit::zeroed()``, - so this would be just a cleanup. - * ``c"" literals`` (stable in 1.77.0). QEMU provides a ``c_str!()`` macro to define ``CStr`` constants easily diff --git a/rust/hw/timer/hpet/src/fw_cfg.rs b/rust/hw/timer/hpet/src/fw_cfg.rs index bef03727ea3..aa08d283519 100644 --- a/rust/hw/timer/hpet/src/fw_cfg.rs +++ b/rust/hw/timer/hpet/src/fw_cfg.rs @@ -4,7 +4,7 @@ use std::ptr::addr_of_mut; -use qemu_api::{cell::bql_locked, impl_zeroable, zeroable::Zeroable}; +use qemu_api::{cell::bql_locked, zeroable::Zeroable}; /// Each `HPETState` represents a Event Timer Block. The v1 spec supports /// up to 8 blocks. QEMU only uses 1 block (in PC machine). @@ -18,7 +18,7 @@ pub struct HPETFwEntry { pub min_tick: u16, pub page_prot: u8, } -impl_zeroable!(HPETFwEntry); +unsafe impl Zeroable for HPETFwEntry {} #[repr(C, packed)] #[derive(Copy, Clone, Default)] @@ -26,7 +26,7 @@ pub struct HPETFwConfig { pub count: u8, pub hpet: [HPETFwEntry; HPET_MAX_NUM_EVENT_TIMER_BLOCK], } -impl_zeroable!(HPETFwConfig); +unsafe impl Zeroable for HPETFwConfig {} #[allow(non_upper_case_globals)] #[no_mangle] diff --git a/rust/qemu-api/src/zeroable.rs b/rust/qemu-api/src/zeroable.rs index a3415a2ebcc..e0720345495 100644 --- a/rust/qemu-api/src/zeroable.rs +++ b/rust/qemu-api/src/zeroable.rs @@ -4,89 +4,15 @@ /// Encapsulates the requirement that /// `MaybeUninit::<Self>::zeroed().assume_init()` does not cause undefined -/// behavior. This trait in principle could be implemented as just: -/// -/// ``` -/// pub unsafe trait Zeroable: Default { -/// const ZERO: Self = unsafe { ::core::mem::MaybeUninit::<Self>::zeroed().assume_init() }; -/// } -/// ``` -/// -/// The need for a manual implementation is only because `zeroed()` cannot -/// be used as a `const fn` prior to Rust 1.75.0. Once we can assume a new -/// enough version of the compiler, we could provide a `#[derive(Zeroable)]` -/// macro to check at compile-time that all struct fields are Zeroable, and -/// use the above blanket implementation of the `ZERO` constant. +/// behavior. /// /// # Safety /// -/// Because the implementation of `ZERO` is manual, it does not make -/// any assumption on the safety of `zeroed()`. However, other users of the -/// trait could use it that way. Do not add this trait to a type unless -/// all-zeroes is a valid value for the type. In particular, remember that -/// raw pointers can be zero, but references and `NonNull<T>` cannot +/// Do not add this trait to a type unless all-zeroes is a valid value for the +/// type. In particular, raw pointers can be zero, but references and +/// `NonNull<T>` cannot. pub unsafe trait Zeroable: Default { - const ZERO: Self; -} - -/// A macro that acts similarly to [`core::mem::zeroed()`], only is const -/// -/// ## Safety -/// -/// Similar to `core::mem::zeroed()`, except this zeroes padding bits. Zeroed -/// padding usually isn't relevant to safety, but might be if a C union is used. -/// -/// Just like for `core::mem::zeroed()`, an all zero byte pattern might not -/// be a valid value for a type, as is the case for references `&T` and `&mut -/// T`. Reference types trigger a (denied by default) lint and cause immediate -/// undefined behavior if the lint is ignored -/// -/// ```rust compile_fail -/// use const_zero::const_zero; -/// // error: any use of this value will cause an error -/// // note: `#[deny(const_err)]` on by default -/// const STR: &str = unsafe{const_zero!(&'static str)}; -/// ``` -/// -/// `const_zero` does not work on unsized types: -/// -/// ```rust compile_fail -/// use const_zero::const_zero; -/// // error[E0277]: the size for values of type `[u8]` cannot be known at compilation time -/// const BYTES: [u8] = unsafe{const_zero!([u8])}; -/// ``` -/// ## Differences with `core::mem::zeroed` -/// -/// `const_zero` zeroes padding bits, while `core::mem::zeroed` doesn't -#[macro_export] -macro_rules! const_zero { - // This macro to produce a type-generic zero constant is taken from the - // const_zero crate (v0.1.1): - // - // https://docs.rs/const-zero/latest/src/const_zero/lib.rs.html - // - // and used under MIT license - ($type_:ty) => {{ - const TYPE_SIZE: ::core::primitive::usize = ::core::mem::size_of::<$type_>(); - union TypeAsBytes { - bytes: [::core::primitive::u8; TYPE_SIZE], - inner: ::core::mem::ManuallyDrop<$type_>, - } - const ZERO_BYTES: TypeAsBytes = TypeAsBytes { - bytes: [0; TYPE_SIZE], - }; - ::core::mem::ManuallyDrop::<$type_>::into_inner(ZERO_BYTES.inner) - }}; -} - -/// A wrapper to implement the `Zeroable` trait through the `const_zero` macro. -#[macro_export] -macro_rules! impl_zeroable { - ($type:ty) => { - unsafe impl $crate::zeroable::Zeroable for $type { - const ZERO: Self = unsafe { $crate::const_zero!($type) }; - } - }; + const ZERO: Self = unsafe { ::core::mem::MaybeUninit::<Self>::zeroed().assume_init() }; } // bindgen does not derive Default here @@ -97,13 +23,13 @@ fn default() -> Self { } } -impl_zeroable!(crate::bindings::Property__bindgen_ty_1); -impl_zeroable!(crate::bindings::Property); -impl_zeroable!(crate::bindings::VMStateFlags); -impl_zeroable!(crate::bindings::VMStateField); -impl_zeroable!(crate::bindings::VMStateDescription); -impl_zeroable!(crate::bindings::MemoryRegionOps__bindgen_ty_1); -impl_zeroable!(crate::bindings::MemoryRegionOps__bindgen_ty_2); -impl_zeroable!(crate::bindings::MemoryRegionOps); -impl_zeroable!(crate::bindings::MemTxAttrs); -impl_zeroable!(crate::bindings::CharBackend); +unsafe impl Zeroable for crate::bindings::Property__bindgen_ty_1 {} +unsafe impl Zeroable for crate::bindings::Property {} +unsafe impl Zeroable for crate::bindings::VMStateFlags {} +unsafe impl Zeroable for crate::bindings::VMStateField {} +unsafe impl Zeroable for crate::bindings::VMStateDescription {} +unsafe impl Zeroable for crate::bindings::MemoryRegionOps__bindgen_ty_1 {} +unsafe impl Zeroable for crate::bindings::MemoryRegionOps__bindgen_ty_2 {} +unsafe impl Zeroable for crate::bindings::MemoryRegionOps {} +unsafe impl Zeroable for crate::bindings::MemTxAttrs {} +unsafe impl Zeroable for crate::bindings::CharBackend {} -- 2.49.0