[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: Refactor arm64/domctl.c 'subarch_do_domctl' to avoid unreachable break.
On 23/10/2023 17:00, Julien Grall wrote: On 23/10/2023 15:51, Nicola Vetrini wrote:Hi,Hi Nicola,while taking care of some patches regarding MISRA C Rule 2.1 (code shouldn't be unreachable), Icame across this function: long subarch_do_domctl(struct xen_domctl *domctl, struct domain *d, XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl) { switch ( domctl->cmd ) { case XEN_DOMCTL_set_address_size: switch ( domctl->u.address_size.size ) { case 32: if ( !cpu_has_el1_32 ) return -EINVAL; /* SVE is not supported for 32 bit domain */ if ( is_sve_domain(d) ) return -EINVAL; return switch_mode(d, DOMAIN_32BIT); case 64: return switch_mode(d, DOMAIN_64BIT); default: return -EINVAL; } break; default: return -ENOSYS; } }here the break after the innermost switch is clearly unreachable, but it's also guarding a possible fallthrough.I can see a couple of solutions to this: - mark the part after the switch unreachable;- introduce a variable 'long rc' to store the return value, and consequently rework the control flow of all the switches(e.g. rc = -EINVAL and similar);- remove the break, but I consider this a risky move, unless -ENOSYS would be an ok value to be returned if some casefrom the switch above does not have a return statement.- move the nested switch in a separate function, so the code in subarch_do_domctl() can be replaced with: return set_address_size(...);What would be the preferred way of addressing this violation?I would actually prefer the 4th option I suggested. Cheers, Would you mind sending the patch yourself? -- Nicola Vetrini, BSc Software Engineer, BUGSENG srl (https://bugseng.com)
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |