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, 4 Mar 2022 19:48:25 +0100
From:   Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
To:     Joe Perches <joe@...ches.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, 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, but it is a trade-off. If it is close enough, the benefits of
automatic formatting may overcome the downsides.

> 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.

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.

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.

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.

> 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.

Cheers,
Miguel