Re: [XEN PATCH 03/13] xen/arm: address violations of MISRA C:2012 Directive 4.10

[Simone Ballarin <simone.ballarin@bugseng.com>]

On 30/08/23 15:01, Jan Beulich wrote:
> On 30.08.2023 14:53, Simone Ballarin wrote:
>> On 29/08/23 00:10, Julien Grall wrote:
>>> On Mon, 28 Aug 2023 at 09:20, Simone Ballarin <simone.ballarin@bugseng.com>
>>> wrote:
>>>> --- a/xen/arch/arm/include/asm/hypercall.h
>>>> +++ b/xen/arch/arm/include/asm/hypercall.h
>>>> @@ -1,10 +1,10 @@
>>>> +#ifndef __ASM_ARM_HYPERCALL_H__
>>>> +#define __ASM_ARM_HYPERCALL_H__
>>>> +
>>>>    #ifndef __XEN_HYPERCALL_H__
>>>>    #error "asm/hypercall.h should not be included directly - include
>>>> xen/hypercall.h instead"
>>>>    #endif
>>>>
>>>> -#ifndef __ASM_ARM_HYPERCALL_H__
>>>> -#define __ASM_ARM_HYPERCALL_H__
>>>> -
>>>
>>>
>>> I understand that you are trying to fix a misra violation. However, this
>>> feels like it was done on purpose.
>>>
>>> With the new change, you would not always check that the file were included
>>> at the correct place. I am not against this change but this ought to be
>>> explained.
>> I don't think the semantics have changed. Please correct me if I'm wrong.
>>
>> With this change, the only situation where the check is not performed is
>> when __ASM_ARM_HYPERCALL_H__ is defined (i.e. the file has already been
>> successfully included). This implies that if __ASM_ARM_HYPERCALL_H__ is
>> defined, then __XEN_HYPERCALL_H__ is also defined, so the check would be
>> useless.
>>
>> The same thing happened with the code before the change: if I include
>> the file after xen/hypercall.h, the check will always succeed.
> 
> Hmm, I think you're right, but I draw a different conclusion: The check
> fails to work as intended. And this can only be repaired without your
> adjustment.
> 

Ok, I will just deviate these cases.

> Jan

-- 
Simone Ballarin, M.Sc.

Field Application Engineer, BUGSENG (https://bugseng.com)