[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [XEN PATCH 1/3] EFI: address violations of MISRA C Rule 13.6
On 11.09.2024 16:27, Andrew Cooper wrote: > On 11/09/2024 3:10 pm, Jan Beulich wrote: >> On 11.09.2024 15:16, Marek Marczykowski-Górecki wrote: >>> On Wed, Sep 11, 2024 at 02:50:03PM +0200, Jan Beulich wrote: >>>> On 10.09.2024 21:06, Federico Serafini wrote: >>>>> Refactor the code to improve readability >>>> I question this aspect. I'm not the maintainer of this code anymore, so >>>> my view probably doesn't matter much here. >>>> >>>>> and address violations of >>>>> MISRA C:2012 Rule 13.6 ("The operand of the `sizeof' operator shall >>>>> not contain any expression which has potential side effect"). >>>> Where's the potential side effect? Since you move ... >>>> >>>>> --- a/xen/common/efi/runtime.c >>>>> +++ b/xen/common/efi/runtime.c >>>>> @@ -250,14 +250,20 @@ int efi_get_info(uint32_t idx, union xenpf_efi_info >>>>> *info) >>>>> info->cfg.addr = __pa(efi_ct); >>>>> info->cfg.nent = efi_num_ct; >>>>> break; >>>>> + >>>>> case XEN_FW_EFI_VENDOR: >>>>> + { >>>>> + XEN_GUEST_HANDLE_PARAM(CHAR16) vendor_name = >>>>> + guest_handle_cast(info->vendor.name, CHAR16); >>>> .. this out, it must be the one. I've looked at it, yet I can't spot >>>> anything: >>>> >>>> #define guest_handle_cast(hnd, type) ({ \ >>>> type *_x = (hnd).p; \ >>>> (XEN_GUEST_HANDLE_PARAM(type)) { _x }; \ >>>> }) >>>> >>>> As a rule of thumb, when things aren't obvious, please call out the >>>> specific aspect / property in descriptions of such patches. >>> I guess it's because guest_handle_cast() is a macro, yet it's lowercase >>> so looks like a function? >> If Eclair didn't look at the macro-expanded code, it wouldn't even see >> the sizeof(). Hence I don't expect the thing to be mistaken for a function >> call. > > The complaint is a sizeof in guest_handle_okay() being given ({ ... }) > to interpret. > > ({}) can have arbitrary side effects in it, hence the violation. I sincerely hope the tool actually looks inside the ({}). Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |