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

[Dmytro Prokopchuk1 <dmytro_prokopchuk1@xxxxxxxx>]

On 7/9/26 13:50, Jan Beulich wrote:
> 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

Ack.
I'll submit a new version.