On 16.08.2023 15:43, Nicola Vetrini wrote: > On 16/08/2023 13:23, Jan Beulich wrote: >> On 16.08.2023 12:47, Nicola Vetrini wrote: >>> On 16/08/2023 12:31, Jan Beulich wrote: >>>> On 16.08.2023 12:01, Nicola Vetrini wrote: >>>>> 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. >>> >>> There is a deviation already in place. At the moment it just ignores >>> anything below an unreachable ASSERT_UNREACHABLE(), no matter what >>> that >>> macro will expand to. >> >> What do you mean by "in place"? docs/misra/ has nothing, afaics. And >> automation/eclair_analysis/ECLAIR/deviations.ecl is merely filtering >> things out of reports, aiui. (Plus of course there should be a single >> central place where all deviations are recorded, imo.) > > The second statement is not quite correct, as some of the configurations > instruct the > checker how to behave.
Well, I was referring to the one setting relevant here, and I added "aiui" to indicate that I may be misreading what that file specifies. Jan
