linux-kernel - Re: [PATCH] scsi: megaraid: cleanup formatting of megaraid

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 04 Mar 2022 11:28:31 -0800
From:   Joe Perches <joe@...ches.com>
To:     Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
Cc:     Tom Rix <trix@...hat.com>, Finn Thain <fthain@...ux-m68k.org>,
        Konrad Wilhelm Kleine <kkleine@...hat.com>,
        Bart Van Assche <bvanassche@....org>,
        kashyap.desai@...adcom.com, sumit.saxena@...adcom.com,
        shivasharan.srikanteshwara@...adcom.com, jejb@...ux.ibm.com,
        martin.petersen@...cle.com, Nathan Chancellor <nathan@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        megaraidlinux.pdl@...adcom.com, scsi <linux-scsi@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>, llvm@...ts.linux.dev
Subject: Re: [PATCH] scsi: megaraid: cleanup formatting of megaraid

On Fri, 2022-03-04 at 19:48 +0100, Miguel Ojeda wrote:
> On Fri, Mar 4, 2022 at 6:37 PM Joe Perches <joe@...ches.com> wrote:
> > 
> > I rather doubt clang-format will ever be 'close enough'.
> > 
> > A human's sense of 'taste' for reading code is very different than
> > what an automated tool produces.
> 
> Maybe,

Hey again Miguel.

Is that statement really disputable?

> but it is a trade-off. If it is close enough, the benefits of
> automatic formatting may overcome the downsides.

IYO.  I think using an SCCS with better language understanding rather
than a line oriented one could be an improvement.  Such a tool could
allow arbitrary style reformatting at check-in/check-out.

> > Also, try looking at the changes clang-format does on a file chosen
> > at random:
> > 
> > o columnarized definitions -> not columnarized
> > o odd line continuation placement using spaces and not tabs before \
> > o odd array definition layouts
> > o per line definitions with comments poorly laid out
> > o individual line definitions rewrapped
> > o enum definitions on multiple lines compressed to single lines
> > o u8 array definition layouts where the first element has a separate
> >   meaning than the subsequent elements are compressed and made
> >   difficult to understand
> 
> I am not sure what you are trying to show here -- some of these are
> precisely the things that the tool could improve or have already
> improved, and we may just need to use the new option.

All of these existing code are more human readable than the code
reformatted using clang-format.

I used whatever is the latest clang-format here with today's -next.
https://releases.llvm.org/download.html

> For instance, for the columnarized macros case, it is possible to
> align them since clang-format 9. For array of structures, there is
> also a new alignment option since clang-format 13. Etc.

Then perhaps you as the maintainer of the kernel's .clang-format file
could update the entries for those new options.

I believe the minimum clang version is already 11.  Maybe higher.
I don't track clang or use it very much.  The clang version I use
though is 13.

> For the wrapping and related bits, now that the limit on 80 is a bit
> more fuzzy, we could perhaps tweak the penalties to improve the
> decision making.

Please have at it.
But perhaps tweaking will just improve some cases and worsen others.

> In summary, what we should be trying to do is improve the tool
> configuration and tool itself to see if we can get it to be close
> enough to the kernel style to make it more widely used.
> 
> > I think _some_ clang-format output is ok, but the concept of
> > enabling/disabling specific reformatting bits would be quite useful.
> > 
> > And sprinkling "clang-format on/off" lines in the code is not good.
> 
> Definitely, but it is fine in some exceptional cases.

I don't think so.

> > Any control codes that determine when source code layout might be
> > immutable or allowed to be modified could be should be tool name
> > agnostic.
> 
> I don't see why would that be a problem, and I don't understand why we
> would use several different formatting tools (the point is to be
> consistent, after all); but sure, we could propose an alternative
> spelling.

Thanks.

There is no "one true editor".
There will not be "one true source code formatter" either.

cheers not jeers, just keep at it.  Joe