Re: [PATCH 0/4] clang-format with examples

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

Hi Oleksandr,

I really appreciate the examples you provided; they were very
enlightening regarding the way forward.

Today we discussed the topic with the maintainers again, and I believe
we have identified the best approach to take.

Since it is not feasible to use clang-format to make changes to an
entire file at once, and even applying clang-format patch by patch, as
demonstrated in this series, poses challenges, we think that
clang-format is not a suitable solution. Many of the suggested changes
are unwelcome, making its use impractical.

We believe the best approach is to start small and use a tool like
checkpatch.pl to enforce a limited set of coding style rules that we all
agree on. A simple starting point could be enforcing basic rules such as
indentation must be four spaces.

The key idea is that it is better to have a tool that enforces a few
rules correctly rather than one that enforces many rules but some of
them wrong. 

With this in mind, a tool similar to checkpatch.pl that verifies only
the coding style rules explicitly defined in CODING_STYLE would be an
excellent starting point. If we identify inconsistencies within Xen
source files, the maintainers have agreed to review each issue
individually and establish a consensus-based resolution, similar to how
we handled MISRA rules.

It is important to note that we will need to make exceptions for files
imported from Linux, many of which are already listed under
docs/misra/exclude-list.json. We can reuse this exclusion list for
coding style checks, ensuring that it does not interfere with how
exclude-list.json is currently used by ECLAIR for MISRA scans.  

Does this approach make sense? In summary, if we had a version of
checkpatch.pl that enforced the coding style defined in CODING_STYLE
instead of Linux-specific style rules, we would be happy to adopt it and
integrate it into the CI loop. The tool does not necessarily have to be
written in Perl; it could be implemented in another language, such as
Python, if that is more convenient.  

One final note: Please disregard the section "Header inclusion guards"
in CODING_STYLE, as we are aware that it needs updating.  

Cheers,

Stefano


On Sat, 1 Mar 2025, Oleksandr Andrushchenko wrote:
> Hello, everybody!
> 
> In order to continue activity started at [1] I am sending this series
> which allows to manually run coding style checks and does not provide
> any automation yet.
> 
> Only the first patch is the goal of this series and patches 2-4 are for
> example only.
> 
> This proposal disables coding style checks for the project, but xen/
> folder: this is done by providing a global .clang-format at the project
> top level which disables clang-format and only providing .clang-format
> for xen/ folder.
> 
> clang-format version expected to be >15 and the latest tool can be
> installed with:
> python3 -m pip install clang-format
> This allows having the tool without the need to compile or install
> anything else.
> 
> As no automatic code style checks are performed, so all those can be
> run manually:
> 
> - to see changes proposed to the patch being worked on (not committed yet):
> git-clang-format --diff --style=file --verbose
> 
> - to run code formatting on the HEAD patch:
> git-clang-format --style=file --verbose HEAD~1
> 
> Provided xen/.clang-format still has a lot of comments gathered from the
> previous discussions. This is for purpose of better seeing why some of
> the options have their values. Once option values are accepted all those
> comments can be removed.
> 
> Stay safe,
> Oleksandr Andrushchenko
> 
> [1] https://lists.xen.org/archives/html/xen-devel/2025-02/msg00800.html
> 
> Oleksandr Andrushchenko (4):
>   Add .clang-format files to enable manual coding style checks
>   [EXAMPLE] Provide an example of a malformed patch
>   [EXAMPLE] Changes done by clang-format to the previous commit
>   [EXAMPLE] Whole file code style formatting
> 
>  .clang-format              |    5 +
>  tools/xl/xl_info.c         |    6 +-
>  xen/.clang-format          | 1380 ++++++++++++++++++++++++++++++++++++
>  xen/drivers/char/ns16550.c |   13 +-
>  xen/drivers/pci/pci.c      |    8 +-
>  5 files changed, 1397 insertions(+), 15 deletions(-)
>  create mode 100644 .clang-format
>  create mode 100644 xen/.clang-format
> 
> -- 
> 2.25.1
>