[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/9] rust/vmstate: Support varray's num field wrapped in BqlC
From: |
Zhao Liu |
Subject: |
Re: [PATCH 2/9] rust/vmstate: Support varray's num field wrapped in BqlCell |
Date: |
Wed, 16 Apr 2025 20:34:37 +0800 |
> > #[macro_export]
> > macro_rules! if_present {
> > ([$($cond:tt)*]: $($result:tt)*) => { $($result)* };
> > }
> >
> > to expand the array part of the access:
> >
> > assert_field_type!(...
> > $($crate::if_present!([$num]: [0]))?;
>
> This example remind me that I introduced a bug into array part:
>
> let index: usize = v.$num.try_into().unwrap();
> types_must_be_equal::<_, &$ti>(&v.$i[index]);
>
> In the current code, actually it accesses v[num], but when num
> stores the length of the whole array, it will cause index out of bounds.
>
> So for current code, at least it should access `v.i[num - 1]`:
>
> let index: usize = v.$num.try_into().unwrap() - 1; // access the last
> element.
> types_must_be_equal::<_, &$ti>(&v.$i[index]);
I realize that my thinking was wrong here! The `v` (with specific type)
isn't a valid instance, and the variable `num` being passed isn't
correctly initialized. Therefore, checking `num`'s value here is
meaningless; it's enough to just check if the type matches!
> > );
> >
> > With this change, assert_field_type! is nicer and at least the trait you're
> > introducing in assertions.rs goes away...
>
> Yes! Great idea.
>
> Then with your help, we could integrate the array part like:
>
> #[macro_export]
> macro_rules! if_present {
> ([$($cond:tt)*]: $($result:tt)*) => { $($result)* };
> }
>
> ...
>
> #[macro_export]
> macro_rules! assert_field_type {
> ($t:ty, $i:tt, $ti:ty $(, $num:ident)?) => {
> const _: () = {
> #[allow(unused)]
> fn assert_field_type(v: $t) {
> fn types_must_be_equal<T, U>(_: T)
> where
> T: $crate::assertions::EqType<Itself = U>,
> {
> }
>
> let access = v.$i$($crate::if_present!([$num]: [v.$num -
> 1])])?;
So, the correct code should just check array[0] as you said:
let access = v.$i$($crate::if_present!([$num]: [0])])?;
Based on this, there's no need for anything else such as `Into`.
> types_must_be_equal::<_, $ti>(access);
> }
> };
> };
> }
Thanks,
Zhao
[PATCH 4/9] rust/vmstate_test: Fix typo in test_vmstate_macro_array_of_pointer_wrapped(), Zhao Liu, 2025/04/14
[PATCH 6/9] rust/hpet: convert num_timers to u8 type, Zhao Liu, 2025/04/14
[PATCH 3/9] rust/vmstate_test: Test varray with num field wrapped in BqlCell, Zhao Liu, 2025/04/14
[PATCH 5/9] rust/timer: Define NANOSECONDS_PER_SECOND binding as u64, Zhao Liu, 2025/04/14
[PATCH 8/9] rust/hpet: Support migration, Zhao Liu, 2025/04/14