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

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

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."

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