[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [XEN PATCH v3] xen/mm: address violations of MISRA C:2012 Rules 8.2 and 8.3
On Fri, 17 Nov 2023, Federico Serafini wrote: > On 20/10/23 08:35, Jan Beulich wrote: > > 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. > > I'll propose a new patch series where changes are splitted as indicated. > I also noticed a discrepancy between Arm and x86 in the name of the > last parameter of xenmem_add_to_physmap_one(). > Do you have any suggestions about how to solve it? > If we reach an agreement, then I can put the changes related to the mm module > in a single patch. I think it should be "gfn"
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |