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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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.