Re: [PATCH] generic/altp2m: address violations of MISRA C Rule 2.1

[Jan Beulich <jbeulich@xxxxxxxx>]

On 09.07.2026 12:00, Dmytro Prokopchuk1 wrote:
> On 6/22/26 13:29, Dmytro Prokopchuk wrote:
>> On 4/10/26 09:04, Jan Beulich wrote:
>>> On 09.04.2026 19:37, Dmytro Prokopchuk1 wrote:
>>>> MISRA C Rule 2.1 states: "A project shall not contain unreachable code".
>>>>
>>>> In certain build configurations,
>>>
>>> Can you give an example where ...
>> If CONFIG_VM_EVENT is enabled on ARM.
>>>
>>>> the function 'altp2m_vcpu_idx()' is defined
>>>> as an inline function that contains the 'BUG()' macro. This resulted 
>>>> in a
>>>> violation because the 'BUG()' macro makes the function non-returning.
>>>>
>>>> To ensure compliance with MISRA C Rule 2.1, this patch removes the 
>>>> inline
>>>> function implementation and its BUG()-based unreachable code. It is 
>>>> replaced
>>>> with an unconditional function declaration for 'altp2m_vcpu_idx()'.
>>>
>>> ... a declaration is needed? The sole non-x86 reference I see is from
>> Yes, declaration is needed. The file 'common/monitor.c' contains a usage 
>> of the 'altp2m_vcpu_idx()' and compiler must see it to avoid "error: 
>> implicit declaration of function ‘altp2m_vcpu_idx’".
>>> common/monitor.c, and the sole relevant Kconfig option I can spot is
>>> VM_EVENT. When that's off, the file won't be built at all.
>> But when 'CONFIG_VM_EVENT=y' this file is compiled on ARM.
>>>
>>> Further, BUG() and a few more constructs have a dedicated deviation
>>> already in place. I don't mind a useless function to be shrunk (or, as
>>> per above, perhaps even dropped), but the justification then needs to
>>> be different.
>> Well, with Stefano's comment regarding return type, the commit subject 
>> and message could be rewritten like:
>>
>> generic/altp2m: align and simplify altp2m_vcpu_idx()
>>
>> The return type of 'altp2m_vcpu_idx()' in the generic altp2m.h header is
>> currently 'unsigned int', which is inconsistent with its 'uint16_t'
>> return type on x86 and the 'altp2m_idx' member of the monitor structures.
>>
>> To fix this type inconsistency and simplify the header, this patch
>> replaces the static inline implementation of 
>> 'altp2m_vcpu_idx()' (contained a BUG() stub) with a simple function 
>> declaration returning
>> uint16_t.
>>
>> For architectures using the generic altp2m.h header (such as ARM when
>> CONFIG_VM_EVENT is enabled), common code calls to 'altp2m_vcpu_idx()' in 
>> common/monitor.c are guarded by 'altp2m_active()', which
>> statically returns 'false'. The compiler's DCE will optimize out these 
>> calls, avoiding any linker issues for the missing definition.
> 
> Hello Jan,
> 
> kind reminder. Please, take a look.

I did, back then already. No specific question was asked for me to answer,
so I was expecting a re-submission, allowing to see new description and
code changes together. That'll then make it easier / more natural to
comment on.

Jan