Re: [XEN PATCH v2 07/13] x86/hvm: address violations of MISRA C Rule 16.3

[Jan Beulich <jbeulich@xxxxxxxx>]

On 25.06.2024 09:21, Federico Serafini wrote:
> On 24/06/24 17:32, Jan Beulich wrote:
>> On 24.06.2024 11:04, Federico Serafini wrote:
>>> @@ -2674,6 +2674,7 @@ static int _hvm_emulate_one(struct hvm_emulate_ctxt 
>>> *hvmemul_ctxt,
>>>   
>>>       default:
>>>           ASSERT_UNREACHABLE();
>>> +        break;
>>>       }
>>>   
>>>       if ( hvmemul_ctxt->ctxt.retire.singlestep )
>>> @@ -2764,6 +2765,7 @@ int hvm_emulate_one_mmio(unsigned long mfn, unsigned 
>>> long gla)
>>>           /* fallthrough */
>>
>> What about this? It doesn't match anything I see in deviations.rst.
> 
> The last item for R16.3 in deviations.rst explicitly says that
> existing comment of this form are considered as safe (i.e., deviated)
> but deprecated, meaning that the pseudo keyword should be used for new
> cases. We can consider a rephrasing if it is not clear enough.

Apologies. I mistakenly looked at grep output rather than actual file
contents. Please disregard this comment of mine.

>>> @@ -5283,6 +5287,8 @@ void hvm_get_segment_register(struct vcpu *v, enum 
>>> x86_segment seg,
>>>            * %cs and %tr are unconditionally present.  SVM ignores these 
>>> present
>>>            * bits and will happily run without them set.
>>>            */
>>> +        fallthrough;
>>> +
>>>       case x86_seg_cs:
>>>           reg->p = 1;
>>>           break;
>>
>> Why the extra blank line here, ...
>>
>>> --- a/xen/arch/x86/hvm/hypercall.c
>>> +++ b/xen/arch/x86/hvm/hypercall.c
>>> @@ -111,6 +111,7 @@ int hvm_hypercall(struct cpu_user_regs *regs)
>>>       case 8:
>>>           eax = regs->rax;
>>>           /* Fallthrough to permission check. */
>>> +        fallthrough;
>>>       case 4:
>>>       case 2:
>>>           if ( currd->arch.monitor.guest_request_userspace_enabled &&
>>
>> ... when e.g. here there's none? I'm afraid this goes back to an
>> unfinished discussion as to whether to have blank lines between blocks
>> in fall-through situations. My view is that not having them in these
>> cases is helping to make the falling through visually noticeable.
> 
> I looked ad the context to preserve the style
> of the existing cases.
> 
> What do you think about:
> -keep the existing style when a break needs to be inserted;

Even that may be a judgment call, I'd say. But commonly: Yes.

> -no blank line if a fallthrough needs to inserted.

Yes here, but others (Andrew?) may disagree with me.

Jan