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

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Thu, Jul 18, 2019 at 7:52 AM Tamas K Lengyel <tamas@xxxxxxxxxxxxx> wrote:
>
> 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.
>

Fortunately I found an astlye option that lets me override the bsd
style in this regard and keep the indentation for these blocks, so it
can still be automated. The only part I can't find an option to is to
incorporate the Xen exception in the do-while style of writing "do {".
I can keep the while-part in the same line but not the brace with the
do. If could make an exception for that in the CODING_STYLE, then the
whole thing could be automated with the following .astylerc I'm using
at the moment:

style=bsd
suffix=none
align-pointer=name
align-reference=name
indent=spaces=4
max-code-length=80
min-conditional-indent=0
attach-closing-while
remove-braces
indent-switches
break-blocks
break-one-line-headers
keep-one-line-blocks
pad-comma
pad-header

Tamas

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