On 8/19/25 16:25, Jan Beulich wrote: > On 19.08.2025 15:12, Dmytro Prokopchuk1 wrote: >> MISRA C Rule 2.1 states: "A project shall not contain unreachable code." >> >> The function 'PrintErrMesg()' is implemented to never return control to >> its caller. At the end of its execution, it calls 'blexit()', which, in >> turn, invokes '__builtin_unreachable()'. This makes the 'return false;' >> statement in 'read_file()' function unreachable. > > I'm disappointed. In earlier review comments I pointed out that there are > two. Yet you say "the", without further disambiguation. > >> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl >> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl >> @@ -41,6 +41,10 @@ not executable, and therefore it is safe for them to be >> unreachable." >> >> -call_properties+={"name(__builtin_unreachable)&&stmt(begin(any_exp(macro(name(ASSERT_UNREACHABLE)))))", >> {"noreturn(false)"}} >> -doc_end >> >> +-doc_begin="Unreachability caused by the call to the 'PrintErrMesg()' >> function is deliberate, as it terminates execution, ensuring no control flow >> continues past this point." >> +-config=MC3A2.R2.1,reports+={deliberate, "any_area(^.*PrintErrMesg.*$ && >> any_loc(file(^xen/common/efi/boot\\.c$)))"} >> +-doc_end > > I don't understand the description here, nor ... > >> --- a/docs/misra/deviations.rst >> +++ b/docs/misra/deviations.rst >> @@ -97,6 +97,13 @@ Deviations related to MISRA C:2012 Rules: >> Xen expects developers to ensure code remains safe and reliable in >> builds, >> even when debug-only assertions like `ASSERT_UNREACHABLE() are >> removed. >> >> + * - R2.1 >> + - Function `PrintErrMesg()` terminates execution (at the end it calls >> + `blexit()`, which, in turn, invokes `__builtin_unreachable()`), >> ensuring >> + no code beyond this point is ever reached. This guarantees that >> execution >> + won't incorrectly proceed or introduce unwanted behavior. >> + - Tagged as `deliberate` for ECLAIR. > > .. the text here. PrintErrMesg() is noreturn. Why would anything need saying > about > it? Isn't the problem here solely with the tail of read_file(), while other > uses > of PrintErrMesg() are okay? > > Jan
I'm a little bit confused. As I understood you proposed to insert the SAF comment before the 'return' statement (with proper justification). And current Eclair configuration & descriptions are not good at all. Am I right? Dmytro.