Re: [XEN PATCH 3/7] xen/arm: address MISRA C:2012 Rule 2.1

From: Nicola Vetrini <nicola.vetrini@bugseng.com>
To: Julien Grall <julien@xen.org>
Cc: Michal Orzel <michal.orzel@amd.com>,
	xen-devel@lists.xenproject.org, consulting@bugseng.com,
	Stefano Stabellini <sstabellini@kernel.org>,
	Bertrand Marquis <bertrand.marquis@arm.com>,
	Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
Subject: Re: [XEN PATCH 3/7] xen/arm: address MISRA C:2012 Rule 2.1
Date: Mon, 11 Dec 2023 15:52:11 +0100	[thread overview]
Message-ID: <cf641b4a6bc31b802d6c1fe92e184a75@bugseng.com> (raw)
In-Reply-To: <731ed3e1-bbf5-4309-90ec-e9f8e7be6620@xen.org>

On 2023-12-11 15:14, Julien Grall wrote:
> On 11/12/2023 13:06, Michal Orzel wrote:
>> 
>> 
>> On 11/12/2023 13:29, Julien Grall wrote:
>>> 
>>> 
>>> Hi,
>>> 
>>> On 11/12/2023 10:30, Nicola Vetrini wrote:
>>>> The "return 1;" statements at the end of some cases in the switch
>>>> of function 'vgic_v3_its_mmio_write' in 'vcig-v3-its.c' cause the
>>>> unreachability of the "return 1;" statement after the switch, thus
>>>> violating MISRA C:2012 Rule 2.1:
>>>> "A project shall not contain unreachable code".
>>>> 
>>>> The same is true for the switch in 'arch_memory_op' from
>>>> 'xen/arch/arm/mm.c'.
>>> 
>>> For both cases, I actually much prefer the "return" version in the
>>> cases. In particular for the vGIC emulation the switch is quite large
>>> and it would not be trivial to know what happens after the break.
>> Because of this...
>> 
>>> 
>>> IOW, I would much prefer if we remove the "return ..." outside of the
>>> switch.
>> wouldn't it be better to add ASSERT_UNREACHABLE() before this return 
>> instead of removing it?
>> This is what we have in e.g. vpl011 and it prevents mistakes.
> 
> I am ok with that. But I am not sure if this would make ECLAIR unhappy.
> 
> Cheers,

Should be okay. I'll test and report back

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)