[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), I
came 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 case
   from 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)



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.