hypfs: add ASSERT_UNREACHABLE() in hypfs_get_entry_rel()

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

Posted by Dmytro Prokopchuk1 6 days, 14 hours ago

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@epam.com>
---
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@epam.com/
---
 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);
 }
 
-- 
2.43.0

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

Posted by Andrew Cooper 5 days, 23 hours ago

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@epam.com>
> ---
> 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@epam.com/
> ---
>  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

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

Posted by Jan Beulich 1 day, 3 hours ago

On 29.04.2026 11: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@epam.com>
>> ---
>> 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@epam.com/
>> ---
>>  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.

Why?

> Not to mention that you are *definitely* not fixing the stated MISRA rule.

Correct. It is instead deviating that rule for this code instance. I'm pretty
sure we have a few other similar pieces of code elsewhere.

In any event - your reply isn't really actionable. If you want things done
differently, then please make a concrete suggestion. Otherwise, in a couple
of days, I'll commit this with Jürgen's R-b.

Jan

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

Posted by Dmytro Prokopchuk1 5 days, 19 hours ago


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@epam.com>
>> ---
>> 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@epam.com/
>> ---
>>   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.

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

Posted by Jürgen Groß 6 days ago

On 28.04.26 20:19, 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@epam.com>

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen