Re: [PATCH v2] hypfs: add ASSERT_UNREACHABLE() in hypfs_get_entry_rel()

[Dmytro Prokopchuk1 <dmytro_prokopchuk1@xxxxxxxx>]

On 4/29/26 12:44, Andrew Cooper wrote:
> On 28/04/2026 7:19 pm, Dmytro Prokopchuk1 wrote:
>> The statement 'return ERR_PTR(-ENOENT);' on the final line of the
>> function 'hypfs_get_entry_rel()' is unreachable because the logic within
>> the infinite loop 'for (;;)' provides all possible exit paths for the
>> function. So there is no execution path to exit the loop and reach the
>> final that statement.
>>
>> This unreachable code violates MISRA C Rule 2.1 which states: "A project
>> shall not contain unreachable code".
>>
>> To fix that and potential compilers "control reaches end of non-void
>> function" warning, mark the code path as unreachable using macro
>> 'ASSERT_UNREACHABLE()'.
>>
>> Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@xxxxxxxx>
>> ---
>> Changes in v2:
>> - add ASSERT_UNREACHABLE() before the final return statement instead of 
>> removing it
>> Test CI pipeline:
>> https://gitlab.com/xen-project/people/dimaprkp4k/xen/-/pipelines/2485661910
>> Link to v1:
>> https://patchew.org/Xen/341811ced2943fb79d0235c27781c564c7bdaf02.1775749146.git.dmytro._5Fprokopchuk1@xxxxxxxx/
>> ---
>>   xen/common/hypfs.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/xen/common/hypfs.c b/xen/common/hypfs.c
>> index cdf4ee0171..02fb234568 100644
>> --- a/xen/common/hypfs.c
>> +++ b/xen/common/hypfs.c
>> @@ -348,6 +348,7 @@ static struct hypfs_entry *hypfs_get_entry_rel(struct 
>> hypfs_entry_dir *dir,
>>           dir = container_of(entry, struct hypfs_entry_dir, e);
>>       }
>>
>> +    ASSERT_UNREACHABLE();
>>       return ERR_PTR(-ENOENT);
>>   }
>>
>
> No.  This is absurd.
>
> Not to mention that you are *definitely* not fixing the stated MISRA rule.
>
> ~Andrew

Hello Andrew.

Looks like there is a conflict between compiler and Eclair.

A compiler is smart enough (should be at least) to understand that
"return ERR_PTR(-ENOENT);" is unreachable in this case, and remove it
during DCE, and at the same time to ignore *potential* warning, that
non-void function doesn't have return statement at the end.

The ECLAIR scans code after preprocessing, so "return ERR_PTR(-ENOENT);"
is still there --> violation.

With "ASSERT_UNREACHABLE();" we just make Eclair happy.

In the patch v1 the return statement was removed. Do you think it is OK?
Making assumption that we will never get compiler error
[-Werror=return-type]...

Dmytro.