On 11.08.2025 23:21, Julien Grall wrote: > On 11/08/2025 21:30, Dmytro Prokopchuk1 wrote: >> --- a/xen/common/grant_table.c >> +++ b/xen/common/grant_table.c >> @@ -330,9 +330,12 @@ shared_entry_header(struct grant_table *t, grant_ref_t >> ref) >> /* Returned values should be independent of speculative execution >> */ >> block_speculation(); >> return &shared_entry_v2(t, ref).hdr; >> + >> + default: >> + ASSERT_UNREACHABLE(); >> + break; >> } >> >> - ASSERT_UNREACHABLE(); > > block_speculation();> >> return NULL; > > I know you are trying to apply the MISRA rule. But this is odd that you > move the ASSERT_UNREACHABLE() but then code after is still only > reachable from the default. In fact, this is introducing a risk if > someone decides to add a new case but then forgot to return a value. > > By moving the two other lines, the compiler should be able to throw an > error if you forgot a return.
I think we did discuss this pattern in the past. While moving everything up to the "return" into the default: handling will please Eclair / Misra, we'll then end up with no return statement at the end of a non-void function. Beyond being good practice (imo) to have such a "main" return statement, that's actually another rule, just one we apparently didn't accept (15.5). Jan
