Re: [XEN PATCH v3] xen/mm: address violations of MISRA C:2012 Rules 8.2 and 8.3

[Jan Beulich <jbeulich@xxxxxxxx>]

On 19.10.2023 18:26, Stefano Stabellini wrote:
> On Thu, 19 Oct 2023, Jan Beulich wrote:
>> On 19.10.2023 00:43, Stefano Stabellini wrote:
>>> On Mon, 16 Oct 2023, Jan Beulich wrote:
>>>> On 03.10.2023 17:24, Federico Serafini wrote:
>>>>> --- a/xen/arch/x86/mm.c
>>>>> +++ b/xen/arch/x86/mm.c
>>>>> @@ -5901,17 +5901,17 @@ int destroy_xen_mappings(unsigned long s, 
>>>>> unsigned long e)
>>>>>   * a problem.
>>>>>   */
>>>>>  void init_or_livepatch modify_xen_mappings_lite(
>>>>> -    unsigned long s, unsigned long e, unsigned int _nf)
>>>>> +    unsigned long s, unsigned long e, unsigned int nf)
>>>>>  {
>>>>> -    unsigned long v = s, fm, nf;
>>>>> +    unsigned long v = s, fm, flags;
>>>>
>>>> While it looks correct, I consider this an unacceptably dangerous
>>>> change: What if by the time this is to be committed some new use of
>>>> the local "nf" appears, without resulting in fuzz while applying the
>>>> patch? Imo this needs doing in two steps: First nf -> flags, then
>>>> _nf -> nf.
>>>
>>> Wouldn't it be sufficient for the committer to pay special attention
>>> when committing this patch? We are in code freeze anyway, the rate of
>>> changes affecting staging is low.
>>
>> Any kind of risk excludes a patch from being a 4.18 candidate, imo.
> 
> I agree on that. I think it is best to commit it for 4.19 when the tree
> opens.
> 
> 
>> That was the case in early RCs already, and is even more so now. Paying
>> special attention is generally a possibility, yet may I remind you that
>> committing in general is intended to be a purely mechanical operation?
> 
> Sure, and I am not asking for a general process change. I am only
> suggesting that this specific concern on this patch is best solved in
> the simplest way: by a committer making sure the patch is correct on
> commit. It is meant to save time for everyone.
> 
> Jan, if you are OK with it, we could just trust you to commit it the
> right away as the earliest opportunity.

If you can get Andrew or Roger to ack this patch in its present shape,
I won't stand in the way. I'm not going to ack the change without the
indicated split.

Jan