On 8/21/25 17:26, Jan Beulich wrote: > 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
Anyway, comments "Doesn't return." and "not reached" are almost the same. To simplify patch, I'm going to leave old comment "not reached" and move description into commit message. Dmytro. > >> The longer explanation may be put in the commit message. >> >> With that addressed: >> >> Reviewed-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com> >> >
