Re: [PATCH 6/8] RDMA/devices: Use xarray to store the clients

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Feb 05, 2019 at 08:46:56AM -0800, Matthew Wilcox wrote:
> On Tue, Feb 05, 2019 at 09:07:45AM -0700, Jason Gunthorpe wrote:
> > On Tue, Feb 05, 2019 at 04:19:14PM +0200, Gal Pressman wrote:
> > > > -	list_for_each_entry(client, &client_list, list)
> > > > +	xa_for_each_marked (&clients, index, client, CLIENT_REGISTERED)
> > > 
> > > nit: extra space after xa_for_each_marked.
> > 
> > meh. Some places in the kernel put the space, maby don't. In this case
> > clang-format wants to put the space for all 'for' macros (and who can
> > blame, it for should aways have a space). I hate to fight with
> > clang-format over such trivial things. Especially when there isn't a
> > specific requirement in coding style.
> 
> Aha.  Apparently I should add this:
> 
> +++ b/.clang-format
> @@ -371,6 +371,12 @@ ForEachMacros:
>    - 'v4l2_m2m_for_each_dst_buf_safe'
>    - 'v4l2_m2m_for_each_src_buf'
>    - 'v4l2_m2m_for_each_src_buf_safe'
> +  - 'xa_for_each'
> +  - 'xa_for_each_marked'
> +  - 'xa_for_each_start'
> +  - 'xas_for_each'
> +  - 'xas_for_each_conflict'
> +  - 'xas_for_each_marked'
>    - 'zorro_for_each_dev'

That won't fix the ' ' - this actually causes the space, since clang
says a for macro and a for statement should have the same layout.

clang-format makes a mess of for macros if they are not in this list
as it treats the expression like a function call, so the '{' placement
is wrong for the for-loop case..

It would be friendly to keep this list updated, I already sent a patch
that is in -rc5, but it doesn't have the latest xa changes.

> As a non-clang user, I didn't ever bother with this file.

Using clang-format via the editor as an 'interactive' single
expression indentor is a big, big time saver, IMHO.

Mainly because it does a quite good job, ie run in whole file mode:

  clang-format -i lib/xarray.c && git diff

To see how it disagrees with the style you used in this file..

I'd say the minor changes around the start of wrapped lines are more
in line with the general kernel consensus, otherwise it already has a
very close agreement to your own style.

Jason



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux