[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 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? Wasn't there some other MISRA rule about lowercase/uppercase for macro names? And yes, I don't really see why this would violate the side effect rule either. -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab Attachment:
signature.asc
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |