Re: [XEN PATCH 07/11] xen: address MISRA C:2012 Rule 2.1

[Nicola Vetrini <nicola.vetrini@bugseng.com>]

Hi,

On 08/08/2023 11:03, Nicola Vetrini wrote:
> On 04/08/2023 08:42, Jan Beulich wrote:
>> On 04.08.2023 01:50, Stefano Stabellini wrote:
>>> On Thu, 3 Aug 2023, Jan Beulich wrote:
>>>> On 02.08.2023 16:38, Nicola Vetrini wrote:
>>>>> Rule 2.1 states: "A project shall not contain unreachable code".
>>>>> 
>>>>> The functions
>>>>> - machine_halt
>>>>> - maybe_reboot
>>>>> - machine_restart
>>>>> are not supposed to return, hence the following break statement
>>>>> is marked as intentionally unreachable with the 
>>>>> ASSERT_UNREACHABLE()
>>>>> macro to justify the violation of the rule.
>>>> 
>>>> During the discussion it was mentioned that this won't help with
>>>> release builds, where right now ASSERT_UNREACHABLE() expands to
>>>> effectively nothing. You want to clarify here how release builds
>>>> are to be taken care of, as those are what eventual certification
>>>> will be run against.
>>> 
>>> Something along these lines:
>>> 
>>> ASSERT_UNREACHABLE(), not only is used in non-release builds to 
>>> actually
>>> assert and detect errors, but it is also used as a marker to tag
>>> unreachable code. In release builds ASSERT_UNREACHABLE() doesn't 
>>> resolve
>>> into an assert, but retains its role of a code marker.
>>> 
>>> Does it work?
>> 
>> Well, it states what is happening, but I'm not convinced it satisfies
>> rule 2.1. There's then still code there which isn't reachable, and
>> which a scanner will spot and report.
>> 
>> Jan
> 
> It's not clear to me whether you dislike the patch itself or the commit
> message. If it's the latter, how about:
> "ASSERT_UNREACHABLE() is used as a marker for intentionally
> unreachable code, which
> constitutes a motivated deviation from Rule 2.1. Additionally, in 
> non-release
> builds, this macro performs a failing assertion to detect errors."

Any feedback on this (with one edit: s/a failing assertion/an 
assertion/)

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