Re: [PATCH v2 05/26] xen/riscv: introduce guest riscv,isa string

[Jan Beulich <jbeulich@xxxxxxxx>]

On 19.05.2026 18:21, Oleksii Kurochko wrote:
> On 5/19/26 5:56 PM, Jan Beulich wrote:
>> On 19.05.2026 17:17, Oleksii Kurochko wrote:
>>> On 5/19/26 4:53 PM, Jan Beulich wrote:
>>>> On 19.05.2026 16:49, Oleksii Kurochko wrote:
>>>>> int init_guest_isa(struct domain *d)
>>>>> {
>>>>>        int len;
>>>>>
>>>>>        bitmap_andnot(d->arch.isa, riscv_isa, guest_unsupp,
>>>>>                      RISCV_ISA_EXT_MAX);
>>>>>
>>>>>        len = build_guest_isa_str(NULL, 0, d->arch.isa);
>>>>>        if ( len < 0 )
>>>>>            return len;
>>>>>
>>>>>        d->arch.isa_str = xmalloc_array(char, len + 1);
>>>>>        if ( !d->arch.isa_str )
>>>>>            return -ENOMEM;
>>>>>
>>>>>        build_guest_isa_str(d->arch.isa_str, len + 1, d->arch.isa);
>>>>
>>>> At least ASSERT() the success of this?
>>>
>>> I will add:
>>>
>>> ASSERT(build_guest_isa_str(d->arch.isa_str, len + 1, d->arch.isa) == len);
>>
>> Ehem. Please check how ASSERT() works (and the difference to BUG_ON()).
> 
> Condition itself looks correct for ASSERT(). If build_guest_isa_str() 
> returns value equal to len then assert_failed() shouldn't be called.
> 
> Maybe do you mean that it will never fire in release build then yes it 
> should be changed to BUG_ON() and the condition should be inverted:
>   BUG_ON(build_guest_isa_str(d->arch.isa_str, len + 1, d->arch.isa) != len);

No. Unlike in BUG_ON(), you can't use expressions with side effects (i.e.
also function calls, unless they're const/pure) in ASSERT(). That's
true for standard C's assert() as well, i.e. not Xen specific at all.
(We do, however, diverge from assert() in another aspect.)

Jan