Re: [PATCH] misra: add deviation for PrintErrMesg() function

[Jan Beulich <jbeulich@xxxxxxxx>]

On 19.08.2025 16:32, Dmytro Prokopchuk1 wrote:
> On 8/19/25 16:25, Jan Beulich wrote:
>> On 19.08.2025 15:12, Dmytro Prokopchuk1 wrote:
>>> MISRA C Rule 2.1 states: "A project shall not contain unreachable code."
>>>
>>> The function 'PrintErrMesg()' is implemented to never return control to
>>> its caller. At the end of its execution, it calls 'blexit()', which, in
>>> turn, invokes '__builtin_unreachable()'. This makes the 'return false;'
>>> statement in 'read_file()' function unreachable.
>>
>> I'm disappointed. In earlier review comments I pointed out that there are
>> two. Yet you say "the", without further disambiguation.
>>
>>> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
>>> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
>>> @@ -41,6 +41,10 @@ not executable, and therefore it is safe for them to be 
>>> unreachable."
>>>   
>>> -call_properties+={"name(__builtin_unreachable)&&stmt(begin(any_exp(macro(name(ASSERT_UNREACHABLE)))))",
>>>  {"noreturn(false)"}}
>>>   -doc_end
>>>   
>>> +-doc_begin="Unreachability caused by the call to the 'PrintErrMesg()' 
>>> function is deliberate, as it terminates execution, ensuring no control 
>>> flow continues past this point."
>>> +-config=MC3A2.R2.1,reports+={deliberate, "any_area(^.*PrintErrMesg.*$ && 
>>> any_loc(file(^xen/common/efi/boot\\.c$)))"}
>>> +-doc_end
>>
>> I don't understand the description here, nor ...
>>
>>> --- a/docs/misra/deviations.rst
>>> +++ b/docs/misra/deviations.rst
>>> @@ -97,6 +97,13 @@ Deviations related to MISRA C:2012 Rules:
>>>          Xen expects developers to ensure code remains safe and reliable in 
>>> builds,
>>>          even when debug-only assertions like `ASSERT_UNREACHABLE() are 
>>> removed.
>>>   
>>> +   * - R2.1
>>> +     - Function `PrintErrMesg()` terminates execution (at the end it calls
>>> +       `blexit()`, which, in turn, invokes `__builtin_unreachable()`), 
>>> ensuring
>>> +       no code beyond this point is ever reached. This guarantees that 
>>> execution
>>> +       won't incorrectly proceed or introduce unwanted behavior.
>>> +     - Tagged as `deliberate` for ECLAIR.
>>
>> .. the text here. PrintErrMesg() is noreturn. Why would anything need saying 
>> about
>> it? Isn't the problem here solely with the tail of read_file(), while other 
>> uses
>> of PrintErrMesg() are okay?
> 
> I'm a little bit confused.
> 
> As I understood you proposed to insert the SAF comment before the 
> 'return' statement (with proper justification).
> 
> And current Eclair configuration & descriptions are not good at all.

Not sure how that's related, but apart from this, ...

> Am I right?

... yes. Yet how is what you submitted here related to the issue in read_file(),
which may be addressable by a simple SAF comment (as you say in your reply)?

Jan