On 21.08.2025 16:24, Marek Marczykowski-Górecki wrote: > On Thu, Aug 21, 2025 at 01:56:28PM +0000, Dmytro Prokopchuk1 wrote: >> MISRA C Rule 2.1 states: "A project shall not contain unreachable code." >> >> The return statements in the 'read_file()' function is unreachable due >> to function 'PrintErrMesg()' which has 'noreturn' attribute: >> PrintErrMesg(name, ret); >> /* not reached */ >> return false; >> } >> >> No explicit return statement is needed here. Remove the statement and >> write a justification comment instead. No functional changes. >> >> Signed-off-by: Dmytro Prokopchuk <dmytro_prokopch...@epam.com> >> --- >> Link to v2: >> https://patchew.org/Xen/c20a58f24875806adfaf491f9c6eef2ca8682d18.1755711594.git.dmytro._5fprokopch...@epam.com/ >> >> Changes in v3: >> - removed unreachable code instead of deviation >> - updated commit subject and message >> >> Test CI pipeline: >> https://gitlab.com/xen-project/people/dimaprkp4k/xen/-/pipelines/1996439444 >> --- >> xen/common/efi/boot.c | 10 +++++++--- >> 1 file changed, 7 insertions(+), 3 deletions(-) >> >> diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c >> index 50ff1d1bd2..325de05b18 100644 >> --- a/xen/common/efi/boot.c >> +++ b/xen/common/efi/boot.c >> @@ -851,9 +851,13 @@ static bool __init read_file(EFI_FILE_HANDLE >> dir_handle, CHAR16 *name, >> PrintErr(what); >> PrintErr(L" failed for "); >> PrintErrMesg(name, ret); >> - >> - /* not reached */ >> - return false; >> + /* >> + * No explicit return statement is needed here because 'PrintErrMesg()' >> is >> + * marked as 'noreturn', which guarantees that it never returns control >> to >> + * the caller. If the 'noreturn' attribute of 'PrintErrMesg()' is >> removed >> + * in the future, compiler will emit an error about the missing return >> + * statement (build-time safeguard). >> + */ > > I don't think this verbose code comment is needed here. Other similar places > use simply "Doesn't return." next to the function call, or nothing at > all if the function name already suggests it (which IMO is not the case > here).
Or simply keep the comment that was already there? Jan > The longer explanation may be put in the commit message. > > With that addressed: > > Reviewed-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com> >
