On Mon, Mar 17, 2025 at 3:52 PM Zhao Liu <zhao1....@intel.com> wrote:
>
> In C version, VMSTATE_VALIDATE accepts the function pointer, which is
> used to check if some conditions of structure could meet, although the
> C version macro doesn't accept any structure as the opaque type.
>
> But it's hard to integrate VMSTATE_VALIDAE into vmstate_struct, a new
> macro has to be introduced to specifically handle the case corresponding
> to VMSTATE_VALIDATE.
>
> One of the difficulties is inferring the type of a callback by its name
> `test_fn`. We can't directly use `test_fn` as a parameter of
> test_cb_builder__() to get its type "F", because in this way, Rust
> compiler will be too conservative on drop check and complain "the
> destructor for this type cannot be evaluated in constant functions".
>
> Fortunately, PhantomData<T> could help in this case, because it is
> considered to never have a destructor, no matter its field type [*].
>
> The `phantom__()` in the `call_func_with_field` macro provides a good
> example of using PhantomData to infer type. So copy this idea and apply
> it to the `vmstate_validate` macro.
>
> [*]: https://doc.rust-lang.org/std/ops/trait.Drop.html#drop-check
>
> Signed-off-by: Zhao Liu <zhao1....@intel.com>
> ---
> rust/qemu-api/src/vmstate.rs | 57 +++++++++++++++++++++++++++++++++++-
> 1 file changed, 56 insertions(+), 1 deletion(-)
>
> diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
> index bb41bfd291c0..4eefd54ca120 100644
> --- a/rust/qemu-api/src/vmstate.rs
> +++ b/rust/qemu-api/src/vmstate.rs
> @@ -25,9 +25,12 @@
> //! functionality that is missing from `vmstate_of!`.
>
> use core::{marker::PhantomData, mem, ptr::NonNull};
> +use std::os::raw::{c_int, c_void};
>
> pub use crate::bindings::{VMStateDescription, VMStateField};
> -use crate::{bindings::VMStateFlags, prelude::*, qom::Owned,
> zeroable::Zeroable};
> +use crate::{
> + bindings::VMStateFlags, callbacks::FnCall, prelude::*, qom::Owned,
> zeroable::Zeroable,
> +};
>
> /// This macro is used to call a function with a generic argument bound
> /// to the type of a field. The function must take a
> @@ -292,6 +295,14 @@ pub const fn with_varray_multiply(mut self, num: u32) ->
> VMStateField {
> self.num = num as i32;
> self
> }
> +
> + #[must_use]
> + pub const fn with_exist_check(mut self) -> Self {
> + assert!(self.flags.0 == 0);
> + self.flags = VMStateFlags(VMStateFlags::VMS_MUST_EXIST.0 |
> VMStateFlags::VMS_ARRAY.0);
> + self.num = 0; // 0 elements: no data, only run test_fn callback
> + self
> + }
> }
>
> /// This macro can be used (by just passing it a type) to forward the
> `VMState`
> @@ -510,6 +521,50 @@ macro_rules! vmstate_fields {
> }}
> }
>
> +pub extern "C" fn rust_vms_test_field_exists<T, F: for<'a> FnCall<(&'a T,
> u8), bool>>(
> + opaque: *mut c_void,
> + version_id: c_int,
> +) -> bool {
> + let owner: &T = unsafe { &*(opaque.cast::<T>()) };
> + let version: u8 = version_id.try_into().unwrap();
> + // SAFETY: the opaque was passed as a reference to `T`.
> + F::call((owner, version))
> +}
> +
> +pub type VMSFieldExistCb = unsafe extern "C" fn(
> + opaque: *mut std::os::raw::c_void,
> + version_id: std::os::raw::c_int,
> +) -> bool;
> +
> +#[doc(alias = "VMSTATE_VALIDATE")]
> +#[macro_export]
> +macro_rules! vmstate_validate {
> + ($struct_name:ty, $test_name:expr, $test_fn:expr $(,)?) => {
> + $crate::bindings::VMStateField {
> + name: ::std::ffi::CStr::as_ptr($test_name),
> + // TODO: Use safe callback.
Why is the TODO still there?
> + field_exists: {
> + const fn test_cb_builder__<
> + T,
> + F: for<'a> $crate::callbacks::FnCall<(&'a T, u8), bool>,
> + >(
> + _phantom: ::core::marker::PhantomData<F>,
> + ) -> $crate::vmstate::VMSFieldExistCb {
> + let _: () = F::ASSERT_IS_SOME;
> + $crate::vmstate::rust_vms_test_field_exists::<T, F>
> + }
> +
> + const fn phantom__<T>(_: &T) ->
> ::core::marker::PhantomData<T> {
> + ::core::marker::PhantomData
> + }
> + Some(test_cb_builder__::<$struct_name,
> _>(phantom__(&$test_fn)))
> + },
> + ..$crate::zeroable::Zeroable::ZERO
> + }
> + .with_exist_check()
> + };
Would it be possible, or make sense, to move most of the code for
field_exists inside .with_exist_check()?
Paolo
