On 16.08.2023 12:01, Nicola Vetrini wrote: > 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. >> >> 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/)
The patch here is kind of okay, but I'm afraid I view my earlier question as not addressed: How will the proposed change prevent the scanner from spotting issues here in release builds? Just stating in the description that there's a deviation is not going to help that. Jan
