Re: [Xen-devel] [PATCH v6 5/5] x86/mem_sharing: style cleanup

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Thu, Jul 18, 2019 at 7:37 AM Jan Beulich <JBeulich@xxxxxxxx> wrote:
>
> On 18.07.2019 15:16, Tamas K Lengyel wrote:
> > On Thu, Jul 18, 2019 at 7:14 AM Jan Beulich <JBeulich@xxxxxxxx> wrote:
> >>
> >> On 18.07.2019 14:59, Tamas K Lengyel wrote:
> >>> On Thu, Jul 18, 2019 at 4:56 AM Jan Beulich <JBeulich@xxxxxxxx> wrote:
> >>>>
> >>>> On 17.07.2019 21:33, Tamas K Lengyel wrote:
> >>>>> @@ -136,8 +137,8 @@ static inline bool _page_lock(struct page_info 
> >>>>> *page)
> >>>>>                 cpu_relax();
> >>>>>             nx = x + (1 | PGT_locked);
> >>>>>             if ( !(x & PGT_validated) ||
> >>>>> -             !(x & PGT_count_mask) ||
> >>>>> -             !(nx & PGT_count_mask) )
> >>>>> +                !(x & PGT_count_mask) ||
> >>>>> +                !(nx & PGT_count_mask) )
> >>>>>                 return false;
> >>>>>         } while ( cmpxchg(&page->u.inuse.type_info, x, nx) != x );
> >>>>
> >>>> Aren't you screwing up indentation here? It looks wrong both in my
> >>>> mail client's view and on the list archives, whereas. Furthermore
> >>>> this is code you've introduced earlier in the series, so it should
> >>>> be got right there, not here.
> >>>
> >>> The style was auto-applied with astyle using the bsd format. In the
> >>> previous patch there were no style-changes applied because it was a
> >>> copy-paste job from the other code location. I rather keep
> >>> code-copying and style fixes separate.
> >>
> >> But you're actively breaking Xen style here (and below).
> >
> > I don't see any mention of style restrictions regarding this in
> > CODING_STYLE. If there is, I would prefer changing that so we can
> > automate style checks which IMHO are the biggest waste of everyone's
> > time to do manually.
>
> ./CODING_STYLE fails to mention many aspects of what we do everywhere.
> Almost any attempt of updating it has failed for me in the past, often
> due to entire lack of responses on patches (in other cases also because
> of people disagreeing). Despite you being the maintainer of the file I
> strongly think you shouldn't actively break style that's in line with
> large swathes of code elsewhere.
>

I wholly disagree. I don't have have time to check for style issues
manually. Patches look like crap to begin with via email and I most
certainly won't bother carving patches out of emails when people fail
to bother to push things as git branches. This should be something
that's done automatically. I don't even think we should be having a
discussions about style issues on the mailinglist. Style fixes could
be made automatically when the patches are applied by the committers.
Anything else is just a waste of time.

Tamas

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel