Re: [PATCH v3] common/efi: fix Rule 2.1 violation in read_file()

[Dmytro Prokopchuk1 <dmytro_prokopchuk1@xxxxxxxx>]

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_prokopchuk1@xxxxxxxx>
>>> ---
>>> Link to v2:
>>> https://patchew.org/Xen/c20a58f24875806adfaf491f9c6eef2ca8682d18.1755711594.git.dmytro._5Fprokopchuk1@xxxxxxxx/
>>>
>>> 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 <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
>>
>