Il ven 1 ago 2025, 17:00 Martin Kletzander <mklet...@redhat.com> ha scritto:
> From: Martin Kletzander <mklet...@redhat.com>
>
> In some cases (e.g. in vmstate_tests.rs) the second argument to
> impl_vmstate_struct! is actually an existing struct which is then
> copied (since VMStateDescription implements Copy) when saved into the
> static VMSD using .get(). That is not a problem because it is part of
> the data segment and the pointers are not being free'd since they point
> to static data. But it is a problem when tests rely on comparing the
> VMState descriptions as pointers rather than contents. And it also
> wastes space, more or less.
>
> Introduce second variant of the macro which can, instead of the
> expression, take an identifier or what looks like a reference. This
> second variant is added before the current variant so that it has
> preference, and only references the existing static data from it.
>
> This way tests are fixed and space is saved.
>
> And now that the VMStateDescription checking is fixed we can also check
> for the right value in test_vmstate_struct_varray_uint8_wrapper().
>
> Signed-off-by: Martin Kletzander <mklet...@redhat.com>
> ---
> I'm not sure whether this is caused by different utility on my system or
> bash
> version or whatever, but without this patch these three tests fail for me
> and
> this patch fixes it.
>
I found something similar, though I wasn't sure if it was broken in master
as well or only in the rust-next branch.
If that works in master as well, I would remove completely the possibility
of using &FOO, and always use .as_ref(). It's more efficient as you said,
and there's no reason that I know to use the less efficient one.
Paolo
rust/qemu-api/src/vmstate.rs | 11 +++++++++++
> rust/qemu-api/tests/vmstate_tests.rs | 1 +
> 2 files changed, 12 insertions(+)
>
> diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
> index b5c6b764fbba..716e52afe740 100644
> --- a/rust/qemu-api/src/vmstate.rs
> +++ b/rust/qemu-api/src/vmstate.rs
> @@ -449,6 +449,17 @@ macro_rules! vmstate_validate {
> /// description of the struct.
> #[macro_export]
> macro_rules! impl_vmstate_struct {
> + ($type:ty, $(&)?$vmsd:ident) => {
> + unsafe impl $crate::vmstate::VMState for $type {
> + const BASE: $crate::bindings::VMStateField =
> + $crate::bindings::VMStateField {
> + vmsd: $vmsd.as_ref(),
> + size: ::core::mem::size_of::<$type>(),
> + flags: $crate::bindings::VMStateFlags::VMS_STRUCT,
> + ..$crate::zeroable::Zeroable::ZERO
> + };
> + }
> + };
> ($type:ty, $vmsd:expr) => {
> unsafe impl $crate::vmstate::VMState for $type {
> const BASE: $crate::bindings::VMStateField = {
> diff --git a/rust/qemu-api/tests/vmstate_tests.rs b/rust/qemu-api/tests/
> vmstate_tests.rs
> index 2c0670ba0eed..7d3180e6c2ea 100644
> --- a/rust/qemu-api/tests/vmstate_tests.rs
> +++ b/rust/qemu-api/tests/vmstate_tests.rs
> @@ -320,6 +320,7 @@ fn test_vmstate_struct_varray_uint8_wrapper() {
> b"arr_a_wrap\0"
> );
> assert_eq!(foo_fields[5].num_offset, 228);
> + assert_eq!(foo_fields[5].vmsd, VMSTATE_FOOA.as_ref());
> assert!(unsafe { foo_fields[5].field_exists.unwrap()(foo_b_p, 0) });
>
> // The last VMStateField in VMSTATE_FOOB.
> --
> 2.50.1
>
>
