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

[Jan Beulich <jbeulich@xxxxxxxx>]

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

> The longer explanation may be put in the commit message.
> 
> With that addressed:
> 
> Reviewed-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
>