* [PATCH 0/4] clang-format with examples
@ 2025-03-01 11:42 Oleksandr Andrushchenko
2025-03-01 11:42 ` [PATCH 1/4] Add .clang-format files to enable manual coding style checks Oleksandr Andrushchenko
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Oleksandr Andrushchenko @ 2025-03-01 11:42 UTC (permalink / raw)
To: xen-devel
Cc: andr2000, sstabellini, Artem_Mygaiev, jbeulich, Luca.Fancellu,
roger.pau, marmarek, andrew.cooper3, anthony.perard
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
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/4] Add .clang-format files to enable manual coding style checks 2025-03-01 11:42 [PATCH 0/4] clang-format with examples Oleksandr Andrushchenko @ 2025-03-01 11:42 ` Oleksandr Andrushchenko 2025-03-03 15:05 ` Roger Pau Monné 2025-03-01 11:42 ` [PATCH 2/4] [EXAMPLE] Provide an example of a malformed patch Oleksandr Andrushchenko ` (3 subsequent siblings) 4 siblings, 1 reply; 10+ messages in thread From: Oleksandr Andrushchenko @ 2025-03-01 11:42 UTC (permalink / raw) To: xen-devel Cc: andr2000, sstabellini, Artem_Mygaiev, jbeulich, Luca.Fancellu, roger.pau, marmarek, andrew.cooper3, anthony.perard, Luca Fancellu Disable coding style checks for the project, but xen/ folder: this is done by providing a global .clang-format at the top level which disables clang-format and only providing .clang-format for xen/. clang-format version expected to be >15 and the latest tool can be installed with: python3 -m pip install clang-format Please note, that no automatic code style checks are performed and 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. Signed-off-by: Oleksandr Andrushchenko <andr2000@gmail.com> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> --- .clang-format | 5 + xen/.clang-format | 1380 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 1385 insertions(+) create mode 100644 .clang-format create mode 100644 xen/.clang-format diff --git a/.clang-format b/.clang-format new file mode 100644 index 000000000000..fc3f8e167f65 --- /dev/null +++ b/.clang-format @@ -0,0 +1,5 @@ +--- +DisableFormat: true +SortIncludes: Never +... + diff --git a/xen/.clang-format b/xen/.clang-format new file mode 100644 index 000000000000..480628544cdd --- /dev/null +++ b/xen/.clang-format @@ -0,0 +1,1380 @@ +# SPDX-License-Identifier: GPL-2.0-only +# +# clang-format configuration file. Intended for clang-format >= 15. +# +# For more information, see: +# +# Documentation/process/clang-format.rst +# https://clang.llvm.org/docs/ClangFormat.html +# https://clang.llvm.org/docs/ClangFormatStyleOptions.html +# +--- + +################################################################################ +# How to review the changes +################################################################################ +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02126.html +# ------------------------------------------------------------------------------ +# George Dunlap: +# It's hard to agree on this one without seeing some of the examples of +# what it does, some examples of the "weird behavior" Stefano & +# Allejandro found, and some examples of places where it's going to +# remove the alignment. +# +# I had tried to apply your series before and didn't get very far with +# it for some reason ISTR. One way to see the effect of individual +# features would be: +# +# 1. Make a branch with one big patch applying clang-format for a given style +# +# 2. Change just one style line, re-run clang format, and create a new +# patch from that +# +# Then it would be easy to see the difference between the two. It might +# actually be easier for individual reviewers to do that on their own +# trees, rather than to ask you to try to generate and post such patches +# somewhere. + +################################################################################ +# do/while loops +################################################################################ +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-07/msg02276.html +# ------------------------------------------------------------------------------ +# Jan: +# > I've read the past threads about the brave people who dared to try to +# > introduce clang-format for the xen codebase, some of them from 5 years ago, +# > two points were clear: 1) goto label needs to be indented and 2) do-while +# > loops have the braket in the same line. +# > While point 1) was quite a blocker, it seemd to me that point 2) was less +# > controversial to be changed in the Xen codestyle, so the current wrapper +# > script handles only the point 1 (which is easy), the point 2 can be more +# > tricky to handle. +# +# I'm afraid I view the do/while part pretty much as a blocker as well. +# While placing the opening brace according to our style elsewhere would +# be okay-ish (just a little wasteful to have two almost empty lines), +# having the closing brace on a separate line is problematic: At least I +# consider a block / scope to end at the line where the closing brace is. +# So the farther do and while are apart, the more +# +# do +# { +# ...; +# } +# while ( cond ); +# ...; +# +# is going to be misleading. While normally we would write potentially +# conflicting constructs this way +# +# while ( cond ) +# ; +# +# the alternative spelling still isn't outright wrong in our style (I +# believe): +# +# while ( cond ); +# +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-07/msg02282.html +# ------------------------------------------------------------------------------ +# Luca: +# Thank you for your feedback, I could maybe misunderstood your reply, so please +# tell me if I am wrong, the Xen coding style mandates this style for do-while +# loops: +# +# do { +# /* Do stuff. */ +# } while ( condition ); +# +# Currently clang-format is able to do only this: +# +# do +# { +# /* Do stuff. */ +# } while ( condition ); +# +# So the issue is only in the opening brackets, not the closing one. Is it a +# blocker too? +# +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-07/msg02284.html +# ------------------------------------------------------------------------------ +# Jan: +# > do +# > { +# > /* Do stuff. */ +# > } while ( condition ); +# +# Oh, I hadn't understood your description that way. +# +# > So the issue is only in the opening brackets, not the closing one. Is it a +# > blocker too? +# +# No. I don't like the longer form, but I could live with it. + +################################################################################ +# clang version to use +################################################################################ +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-07/msg02301.html +# ------------------------------------------------------------------------------ +# Alejandro: +# > The minimum clang-format version for the file is 15, my ubuntu 22.04 comes +# > with it, we can reason if it's too high, or if we could also use the latest +# > version maybe shipped inside a docker image. +# 15 sounds ok \methinks. In practice we'll just stick it in GitLab and let +# it check commits, so we'll be safe even in the case where every developer +# has a slightly different version of the tool. I wouldn't try to make this +# (hard) task harder by trying to retrofit it in an older version. +# +# It might be worth stitching a flag in the python script to scream if the +#clang-format version doesn't match. Or it may do a mess of the tree. + +################################################################################ +# Options and their discussions +################################################################################ + +# [not specified] +# Align function parameter that goes into a new line, under the open bracket +# (supported in clang-format 3.8) +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg01993.html +# ------------------------------------------------------------------------------ +# Luca: This one is to align function parameters that overflows the line length, +# I chose to align them to the open bracket to match the current codebase +# (hopefully) e.g.: +# someLongFunction(argument1, +# argument2); +# +# This one can be a candidate for an entry in our coding style +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02143.html +# ------------------------------------------------------------------------------ +# Jan: +# > e.g.: +# > someLongFunction(argument1, +# > argument2); +# The above matches neither of the two generally permitted styles: +# +# someLongFunction(argument1, +# argument2); +# +# someLongFunction( +# argument1, +# argument2); +# +# Then again from its name I would infer this isn't just about function +# arguments? +# +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02172.html +# ------------------------------------------------------------------------------ +# Luca: +# I think it applies to parameters and arguments of functions and macro, given +# the description in the docs. +# I see your two snippets above but I’ve always found at least on arm a +# predominance of the style above for functions, so arguments aligned after the +# opening bracket, for macros there is a mix. +# I might be wrong though and so another opinion from another maintainer would +# help. +# In any case we can choose among many value: +# https://clang.llvm.org/docs/ClangFormatStyleOptions.html#alignafteropenbracket, +# but when we do so, we need to stick to one permitted style only, the tool don’t +# allow to specify more than one. +# +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02311.html +# ------------------------------------------------------------------------------ +# Jan: +# > I see your two snippets above but I’ve always found at least on arm a +# > predominance of the style above for functions, so arguments aligned after +# > the opening bracket, for macros there is a mix. +# +# The latter "above" refers to which form exactly? The one you originally +# spelled out, or the former of what my reply had? +# +# > In any case we can choose among many value: +# > https://clang.llvm.org/docs/ClangFormatStyleOptions.html#alignafteropenbracket, +# > but when we do so, we need to stick to one permitted style only, the tool +# > don’t allow to specify more than one. +# +# a2k: RED FLAG +# On top of my earlier reply yet another reason perhaps that this tool then +# won't really fit our intended use. +# +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02356.html +# ------------------------------------------------------------------------------ +# Luca: +# > The latter "above" refers to which form exactly? The one you originally +# > spelled out, or the former of what my reply had? +# +# In my reply I was referring to the one I originally spelled out. +# +# > On top of my earlier reply yet another reason perhaps that this tool then +# > won't really fit our intended use. +# ok +# +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02361.html +# ------------------------------------------------------------------------------ +# Jan: +# > In my reply I was referring to the one I originally spelled out. +# +# But that's properly malformed, for not aligning argument2 with argument1. +# Where are such constructs commonly used? I just took one example (cpuerrata.c) +# and only found my first form of wrapping. +# +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02366.html +# ------------------------------------------------------------------------------ +# Luca: +# > But that's properly malformed, for not aligning argument2 with argument1. +# > Where are such constructs commonly used? I just took one example (cpuerrata.c) +# > and only found my first form of wrapping. +# +# I had to go back to the thread to realise there was some style issue, the +# snippet I thought I sent already had argument2 aligned under argument1, like +# this: +# +# e.g.: +# someLongFunction(argument1, +# argument2); +# +# Even my wording was not really accurate, as I meant: +# “This one is to align function parameters that overflows the line length, I +# chose to align them *after* the open bracket to match the current codebase +# (hopefully)” +# So to summarize, this value formats the code to have overflowing args/param +# aligned after the open braket. +# +# But I understand that having only one format is not desirable. +# +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02367.html +# ------------------------------------------------------------------------------ +# Jan: +# > So to summarize, this value formats the code to have overflowing args/param +# > aligned after the open braket. +# +# But the example above _again_ does not have the two arguments aligned. +# I would have suspected a mail UI issue, but +# https://lists.xen.org/archives/html/xen-devel/2023-11/msg02366.html +# similarly shows way too many indenting blanks on the 2nd line. +AlignAfterOpenBracket: Align + +# [not specified] +# Align array of struct's elements by column and justify +# struct test demo[] = +# { +# {56, 23, "hello"}, +# {-1, 93463, "world"}, +# {7, 5, "!!" } +# }; +# (supported in clang-format 13) +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg01993.html +# ------------------------------------------------------------------------------ +# Luca: "When using initialization for an array of structs aligns the fields into +# columns." +# It’s important to say that even if we specify "None", it is going to format the +# data structure anyway, I choose left, but clearly I’m open to suggestions. +# I don’t know how to phrase this one in our coding style +# +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02143.html +# ------------------------------------------------------------------------------ +# Jan: +# You don't say in which way it re-formats such constructs. +# +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02172.html +# ------------------------------------------------------------------------------ +# Luca: +# > You don't say in which way it re-formats such constructs. +# Sure, taking as example an array of structure, xen/drivers/video/modelines.h, +# With AlignArrayOfStructures: None we have this below. +# +# diff --git a/xen/drivers/video/modelines.h b/xen/drivers/video/modelines.h +# index 9cb7cdde055f..3ff23ef1f8a7 100644 +# --- a/xen/drivers/video/modelines.h +# +++ b/xen/drivers/video/modelines.h +# @@ -42,36 +42,36 @@ struct modeline { +# }; +# +# struct modeline __initdata videomodes[] = { +# - { "640x480@60", 25175, 640, 16, 96, 48, 480, 11, 2, 31 }, +# +# [snip] +# +# With AlignArrayOfStructures: Left we have (I noticed there might be a small +# bug in clang-format since the first entry has no space after the opening curly +# bracket): +# +# diff --git a/xen/drivers/video/modelines.h b/xen/drivers/video/modelines.h +# index 9cb7cdde055f..1afe725dcb4c 100644 +# --- a/xen/drivers/video/modelines.h +# +++ b/xen/drivers/video/modelines.h +# @@ -42,36 +42,36 @@ struct modeline { +# }; +# +# struct modeline __initdata videomodes[] = { +# - { "640x480@60", 25175, 640, 16, 96, 48, 480, 11, 2, 31 }, +# +# [snip] +# +# With AlignArrayOfStructures: Right we have: +# +# diff --git a/xen/drivers/video/modelines.h b/xen/drivers/video/modelines.h +# index 9cb7cdde055f..539ab7c12d00 100644 +# --- a/xen/drivers/video/modelines.h +# +++ b/xen/drivers/video/modelines.h +# @@ -42,36 +42,36 @@ struct modeline { +# }; +# +# struct modeline __initdata videomodes[] = { +# - { "640x480@60", 25175, 640, 16, 96, 48, 480, 11, 2, 31 }, +# +# [snip] +# +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02311.html +# ------------------------------------------------------------------------------ +# Jan: +# a2k: RED FLAG +# None of which improve what we have at present, imo. Yet another reason not +# to use this tool then, I would say. +# +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02356.html +# ------------------------------------------------------------------------------ +# Luca: +# > None of which improve what we have at present, imo. Yet another reason not +# > to use this tool then, I would say. +# Ok +AlignArrayOfStructures: Left + +# [not specified] +# Align consecutive assignments (supported in clang-format 3.8) +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg01993.html +# ------------------------------------------------------------------------------ +# Luca: This one is disabled because of feedbacks from Stefano and Alejandro +# about some weird behaviour on our codebase. +# This one could be phased along this line: “Consecutive assignments don't need +# to be aligned.”, the issue is that in this way it seems that it’s optional, +# but clang-format is going to remove the alignment anyway for assignment that +# are consecutive and aligned. +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02126.html +# ------------------------------------------------------------------------------ +# George: It's hard to agree on this one without seeing some of the examples of +# what it does, some examples of the "weird behavior" Stefano & +# Allejandro found, and some examples of places where it's going to +# remove the alignment. +# +# I had tried to apply your series before and didn't get very far with +# it for some reason ISTR. One way to see the effect of individual +# features would be: +# +# 1. Make a branch with one big patch applying clang-format for a given style +# 2. Change just one style line, re-run clang format, and create a new +# patch from that +# +# Then it would be easy to see the difference between the two. It might +# actually be easier for individual reviewers to do that on their own +# trees, rather than to ask you to try to generate and post such patches +# somewhere. +# +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02132.html +# ------------------------------------------------------------------------------ +# Luca: +# > It's hard to agree on this one without seeing some of the examples of +# > what it does, some examples of the "weird behavior" Stefano & +# > Allejandro found, +# I think there was a comment from Stefano for the RFC v1: +# https://patchwork.kernel.org/project/xen-devel/cover/20230728081144.4124309-1-luca.fancellu@xxxxxxx/#25447625 +# +# a2k: +# RFC v1: https://patchew.org/Xen/20230728081144.4124309-1-luca.fancellu@arm.com/ +# +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02143.html +# ------------------------------------------------------------------------------ +# Jan: +# > This one is disabled because of feedbacks from Stefano and Alejandro about +# > some weird behaviour on our codebase. +# > This one could be phased along this line: “Consecutive assignments don't need +# > to be aligned.”, the issue is that in this way it seems that it’s optional, +# > but clang-format is going to remove the alignment anyway for assignment +# > that are consecutive and aligned. +# +# Like below if there's no way to say "leave alone", then I don't consider this +# usable. Imo we want to permit people to align when they think it helps, but +# we don't want to demand them doing so. +# +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02172.html +# ------------------------------------------------------------------------------ +# Luca: +# > Like below if there's no way to say "leave alone", then I don't consider this +# > usable. Imo we want to permit people to align when they think it helps, but +# > we don't want to demand them doing so. +# +# Yes I understand, unfortunately the tool doesn’t allow that and if we don’t +# specify this configurable, it will take the default one which is disabled +# (and so would remove the formatting from already aligned assignments +# that it would eventually find). +# +# This most of the configurable, when not specified, they apply their default +# to the code anyway. +#AlignConsecutiveAssignments: None +# According to https://releases.llvm.org/15.0.0/tools/clang/docs/ClangFormatStyleOptions.html +# Nested configuration flags: Alignment options. +# +# a2k: FIXME: If we use None above then the below options are not needed? +AlignConsecutiveAssignments: + Enabled: false + AcrossEmptyLines: false + AcrossComments: false + +# [not specified] +# Do not align consecutive bit fields (supported in clang-format 11) +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg01993.html +# ------------------------------------------------------------------------------ +# Luca: Same thing as AlignConsecutiveAssignments, but for bitfields. +AlignConsecutiveBitFields: None + +# [not specified] +# Do not align values of consecutive declarations +# (supported in clang-format 3.8) +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg01993.html +# ------------------------------------------------------------------------------ +# Luca: This aligns declarations names, same considerations as +# AlignConsecutiveAssignments. +AlignConsecutiveDeclarations: None + +# [not specified] +# Align values of consecutive macros (supported in clang-format 9) +AlignConsecutiveMacros: + Enabled: true + AcrossEmptyLines: false + AcrossComments: false + +# [not specified] +# Align escaped newlines to the right (supported in clang-format 5) +AlignEscapedNewlines: Right + +# [not specified] +# Aligns operands of a single expression that needs to be split over multiple +# lines (supported in clang-format 3.5) +AlignOperands: Align + +# Do not align trailing consecutive comments (It helps to make clang-format +# reproduce the same output when it runs on an already formatted file) +# (supported in clang-format 3.7) +AlignTrailingComments: true + +# [not specified] +# Do not put all function call arguments on a new line, try to have at least +# the first one close to the opening parenthesis (supported in clang-format 9) +AllowAllArgumentsOnNextLine: false + +# [not specified] +# Do not put all function declaration parameters on a new line, try to have at +# least the first one close to the opening parenthesis +# (supported in clang-format 3.3) +AllowAllParametersOfDeclarationOnNextLine: false + +# Bracing condition needs to be respected even if the line is so short that the +# final block brace can stay on a single line +# (supported in clang-format 3.5) +# Luca: AllowShortBlocksOnASingleLine: Never +# Oleksandr: this is good for empty inline functions, e.g. +# +# -static inline void arm_smmu_disable_ats(struct arm_smmu_master *master) { } +# +static inline void arm_smmu_disable_ats(struct arm_smmu_master *master) +# +{} +AllowShortBlocksOnASingleLine: Empty + +# (supported in clang-format 3.6) +AllowShortCaseLabelsOnASingleLine: false + +# (supported in clang-format 3.11) +AllowShortEnumsOnASingleLine: false + +# (supported in clang-format 3.5) +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-07/msg02355.html +# ------------------------------------------------------------------------------ +# Luca: +# >> static void __init efi_arch_cpu(void) +# >> -{ +# >> -} +# >> +{} +# >> +# +# Also this one can be changed I think by tweaking +# AllowShortFunctionsOnASingleLine, so that we can have just: +# +# static void __init efi_arch_cpu(void) {} +AllowShortFunctionsOnASingleLine: Empty + +# (supported in clang-format 3.3) +AllowShortIfStatementsOnASingleLine: Never + +# (supported in clang-format 3.7) +AllowShortLoopsOnASingleLine: false + +# [not specified] +# Do not add a break after the definition return type +# (supported in clang-format 3.8) +AlwaysBreakAfterReturnType: None + +# [not specified] +# There is no need to use a break after an assigment to a multiline string +# (supported in clang-format 3.4) +AlwaysBreakBeforeMultilineStrings: false + +# (supported in clang-format 3.4) +AlwaysBreakTemplateDeclarations: false + +# Specify Xen's macro attributes (supported in clang-format 12) +# +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg00833.html +# ------------------------------------------------------------------------------ +# Jan: Like above, the significance of having this list and needing to keep it +# up-to-date is unclear to me. I guess the same also applies to a few +# further settings down from here. +# +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg00867.html +# ------------------------------------------------------------------------------ +# Luca: From what I understand, we should give to the formatter tool all the hint +# possible to make it do a good +# job, for example here we should maintain a list of our attributes so that +# clang-format doesn’t think the function +# below is called __init instead of device_tree_node_matches. + +# static bool __init device_tree_node_matches(const void *fdt, int node, const +# char *match) +# { ... } +# +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg00888.html +# ------------------------------------------------------------------------------ +# Jan: Well, perhaps they indeed need to do some level of syntax analysis, in +# which case knowing which identifiers act as attributes is likely going +# to help. Which is where the "needs keeping up-to-date" aspect comes into +# play. The example above is simple enough that I wouldn't think a parser +# needs to guess what this represents, but presumably there are cases +# where ambiguities might arise. (I also wouldn't exclude that the more +# involved syntax in C++ increases the desire to have apriori knowledge on +# the purpose of certain identifiers. In the end, as per above, the tool +# is being told to expect C++ code.) +AttributeMacros: + - '__init' + - '__exit' + - '__initdata' + - '__initconst' + - '__initconstrel' + - '__initdata_cf_clobber' + - '__initconst_cf_clobber' + - '__hwdom_init' + - '__hwdom_initdata' + - '__maybe_unused' + - '__packed' + - '__stdcall' + - '__vfp_aligned' + - '__alt_call_maybe_initdata' + - '__cacheline_aligned' + - '__ro_after_init' + - 'always_inline' + - 'noinline' + - 'noreturn' + - '__weak' + - '__inline__' + - '__attribute_const__' + - '__transparent__' + - '__used' + - '__must_check' + - '__kprobes' + +# [not specified] +# Try always to pack function call arguments on the same line before breaking +# (supported in clang-format 3.7) +BinPackArguments: true + +# [not specified] +# Try always to pack function declaration parameters on the same line before +# breaking (supported in clang-format 3.7) +BinPackParameters: true + +# [not specified] +# Do not add a spaces on bitfield 'unsigned bf:2;' +# (supported in clang-format 12) +BitFieldColonSpacing: None + +# Xen's coding style does not follow clang-format already available profiles for +# breaking before braces, so set it to Custom and specify each case separately +# (supported in clang-format 3.8) +BraceWrapping: + # Braces ('{' and '}') are usually placed on a line of their own + AfterCaseLabel: true + AfterFunction: true + BeforeElse: true + AfterExternBlock: true + # except for the opening brace in definitions of enum, struct, and union + AfterEnum: false + AfterStruct: false + AfterUnion: false + # This is unlike the Linux coding style and unlike K&R. + # + # if ( condition ) + # { + # /* Do stuff. */ + # } + # else + # { + # /* Other stuff. */ + # } + # + # while ( condition ) + # { + # /* Do stuff. */ + # } + # + # [non-compliant] + # do-while is not compliant with CODING_STYLE because clang format doesn't + # support different styles for every control statement + # do + # { + # /* Do stuff. */ + # } while ( condition ); + AfterControlStatement: Always + BeforeWhile: false + IndentBraces: false + # [not specified] + # Keep empty functions with braces style like this: 'void func() {}' instead + # of breaking the braces + SplitEmptyFunction: false + # Not related to C language, but specified for completeness + AfterClass: false + AfterNamespace: false + AfterObjCDeclaration: false + BeforeCatch: false + BeforeLambdaBody: false + SplitEmptyRecord: true + SplitEmptyNamespace: true + +# (supported in clang-format 3.18) +BreakAdjacentStringLiterals: false + +# [not specified] +# Break only after the operator in case of a long expression +# (supported in clang-format 3.6) +BreakBeforeBinaryOperators: None + +# Xen's coding style does not follow clang-format already available profiles for +# breaking before braces, so set it to Custom and specify each case separately +# (supported in clang-format 3.7) +BreakBeforeBraces: Custom + +# [not specified] +# Break before inline ASM colon if the line length is longer than column limit. +# (This is a new feature upstreamed by EPAM during its work on clang-format for +# Xen) +# (supported in clang-format 16) +BreakBeforeInlineASMColon: OnlyMultiline + +# [not specified] +# Ternary operators '?, :' can be put after a line break +# (supported in clang-format 3.7) +BreakBeforeTernaryOperators: true + +# (supported in clang-format 5) +BreakConstructorInitializers: BeforeComma + +# User visible strings (e.g., printk() messages) should not be split so they can +# be searched for more easily. (supported in clang-format 3.9) +BreakStringLiterals: false + +# Lines should be less than 80 characters in length +# (supported in clang-format 3.7) +ColumnLimit: 80 + +# (supported in clang-format 3.7) +CommentPragmas: '^ IWYU pragma:' + +# Code within blocks is indented by one extra indent level +# (supported in clang-format 3.7) +ContinuationIndentWidth: 4 + +# Do not derive pointer alignment style from the file +# (supported in clang-format 3.7) +DerivePointerAlignment: false + +# Taken from: +# git grep -h -i '^#define [^[:space:]]*for_each[^[:space:]]*(' xen/ \ +# | sed "s,^#define \([^[:space:]]*for_each[^[:space:]]*\)(.*$, - '\1',i" \ +# | LC_ALL=C sort -u +# A vector of macros that should be interpreted as foreach loops instead of as +# function calls. +# (supported in clang-format 3.7) +ForEachMacros: + - 'FOR_EACH_IOREQ_SERVER' + - '__list_for_each_rcu' + - 'dt_for_each_child_node' + - 'dt_for_each_device_node' + - 'dt_for_each_property_node' + - 'ebitmap_for_each_positive_bit' + - 'expr_list_for_each_sym' + - 'fdt_for_each_property_offset' + - 'fdt_for_each_subnode' + - 'for_each_affinity_balance_step' + - 'for_each_amd_iommu' + - 'for_each_cfg_sme' + - 'for_each_cpu' + - 'for_each_domain' + - 'for_each_domain_in_cpupool' + - 'for_each_drhd_unit' + - 'for_each_kimage_entry' + - 'for_each_node_mask' + - 'for_each_online_cpu' + - 'for_each_online_node' + - 'for_each_pdev' + - 'for_each_possible_cpu' + - 'for_each_present_cpu' + - 'for_each_rmrr_device' + - 'for_each_sched_unit' + - 'for_each_sched_unit_vcpu' + - 'for_each_set_bit' + - 'for_each_vcpu' + - 'for_each_vp' + - 'hlist_for_each' + - 'hlist_for_each_entry' + - 'hlist_for_each_entry_continue' + - 'hlist_for_each_entry_from' + - 'hlist_for_each_entry_rcu' + - 'hlist_for_each_entry_safe' + - 'hlist_for_each_safe' + - 'list_for_each' + - 'list_for_each_backwards_safe' + - 'list_for_each_continue_rcu' + - 'list_for_each_entry' + - 'list_for_each_entry_continue' + - 'list_for_each_entry_from' + - 'list_for_each_entry_rcu' + - 'list_for_each_entry_reverse' + - 'list_for_each_entry_safe' + - 'list_for_each_entry_safe_continue' + - 'list_for_each_entry_safe_from' + - 'list_for_each_entry_safe_reverse' + - 'list_for_each_prev' + - 'list_for_each_rcu' + - 'list_for_each_safe' + - 'list_for_each_safe_rcu' + - 'page_list_for_each' + - 'page_list_for_each_safe' + - 'page_list_for_each_safe_reverse' + +# A vector of macros that should be interpreted as conditionals instead of as +# function calls. (supported in clang-format 13) +#IfMacros: +# - '' + +# Sort include files by block of includes and not as only one group +# (supported in clang-format 6) +IncludeBlocks: Preserve + +# [not specified] +# Regular expressions denoting the different #include categories used for +# ordering #includes. (supported in clang-format 3.8) +#IncludeCategories: +# - Regex: '.*' +# Priority: 1 + +# When guessing whether a #include is the “main” include (to assign category 0, +# see above), use this regex of allowed suffixes to the header stem. A partial +# match is done, so that: - “” means “arbitrary suffix” - “$” means “no suffix” +# For example, if configured to “(_test)?$”, then a header a.h would be seen as +# the “main” include in both a.cc and a_test.cc. +# (Do we have a main include in Xen?) +# (supported in clang-format 3.9) +#IncludeIsMainRegex: '(Test)?$' + +# Non-case labels inside switch() bodies are preferred to be indented the same +# as the block's case labels (supported in clang-format 11) +IndentCaseBlocks: false + +# [not specified] +# Do not indent case labels in the switch block (supported in clang-format 3.3) +IndentCaseLabels: false + +# [not specified] +# Do not indent extern "C" block +# (supported in clang-format 11) +IndentExternBlock: NoIndent + +# Due to the behavior of GNU diffutils "diff -p", labels should be indented by +# at least one blank (supported in clang-format 10) +IndentGotoLabels: true + +# [not specified] +# Do not create indentation for the preprocessor directives +# (supported in clang-format 6) +IndentPPDirectives: AfterHash + +# An indent level consists of four spaces (supported in clang-format 3.7) +IndentWidth: 4 + +# [not specified] +# In case a function definition or declaration needs to be wrapped after the +# type, do not indent the new line (supported in clang-format 3.7) +IndentWrappedFunctionNames: false + +# Braces should be omitted for blocks with a single statement. e.g., +# if ( condition ) +# single_statement(); +# (supported in clang-format 15) +InsertBraces: false + +# [not specified] +# Every file needs to end with a new line +# (supported in clang-format 16) +# InsertNewlineAtEOF: true + +# [not specified] +# Keep empty lines (up to MaxEmptyLinesToKeep) at end of file +# (supported in clang-format 17) +# KeepEmptyLinesAtEOF: true + +# [not specified] +# Do not add a new empty line at the start of the block +# (supported in clang-format 3.7) +KeepEmptyLinesAtTheStartOfBlocks: false + +# The Xen Hypervisor is written in C language (supported in clang-format 3.5) +Language: Cpp + +# [not specified] +# Line ending style is '\n' (supported in clang-format 16) +# LineEnding: LF + +# Specify Xen's macro that starts a block of code +# (supported in clang-format 3.7) +MacroBlockBegin: '^PLATFORM_START|^DT_DEVICE_START|^ACPI_DEVICE_START' + +# Specify Xen's macro that ends a block of code (supported in clang-format 3.7) +MacroBlockEnd: '^PLATFORM_END|^DT_DEVICE_END|^ACPI_DEVICE_END' + +# [not specified] +# Maximum consecutive empty lines allowed (supported in clang-format 3.7) +MaxEmptyLinesToKeep: 1 + +# Parameters to tweak to discourage clang-format to break something +# (supported in clang-format 5) +PenaltyBreakAssignment: 30 +# (supported in clang-format 3.7) +PenaltyBreakBeforeFirstCallParameter: 30 +# (supported in clang-format 3.7) +PenaltyBreakComment: 10 +# (supported in clang-format 3.7) +PenaltyBreakFirstLessLess: 0 +# (supported in clang-format 14) +PenaltyBreakOpenParenthesis: 100 +# (supported in clang-format 3.7) +PenaltyBreakString: 10 +# (supported in clang-format 3.7) +PenaltyExcessCharacter: 100 +# (supported in clang-format 12) +PenaltyIndentedWhitespace: 0 +# (supported in clang-format 3.7) +PenaltyReturnTypeOnItsOwnLine: 60 + +# [not specified] +# Pointer alignment style is on the right 'void *var' +# (supported in clang-format 3.7) +PointerAlignment: Right + +# [not specified] +# The number of columns to use for indentation of preprocessor statements +# When set to -1 (default) IndentWidth is used also for preprocessor statements. +# (supported in clang-format 13) +PPIndentWidth: -1 + +# [not specified] +# (supported in clang-format 14) +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg01993.html +# ------------------------------------------------------------------------------ +# Luca: +# Our coding style doesn’t mention anything about alignment, shall we add a new +# section? I can send patches when we reach agreement on each of these rules. +# +# QualifierAlignment: Custom +# QualifierOrder: ['static', 'inline', 'const', 'volatile', 'type'] +# +# --- +# For “QualifierAlignment” I chose Custom in order to apply in “QualifierOrder” +# an order for the qualifiers that match the current codebase, we could specify +# also “Leave” in order to keep them as they are. +# Depending on how the discussion goes on this one, it could be an entry in our +# coding style +# +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02143.html +# ------------------------------------------------------------------------------ +# Jan: Where do attributes go in this sequence? +# +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02172.html +# ------------------------------------------------------------------------------ +# Luca: +# > Where do attributes go in this sequence? +# +# I think function declaration/definition and variables. +# +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02311.html +# ------------------------------------------------------------------------------ +# Jan: +# > I think function declaration/definition and variables. +# +# How does this relate to my question? I asked about the sequence of elements +# listed for QualifierOrder:, where attributes don't appear at all right now. +# +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02356.html +# ------------------------------------------------------------------------------ +# Luca: +# > How does this relate to my question? I asked about the sequence of elements +# > listed for QualifierOrder:, where attributes don't appear at all right now. +# +# Sorry, I misread your question, attributes are like invisible for the tool, +# they can be placed wherever between each of the QualifierOrder items. +# +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02361.html +# ------------------------------------------------------------------------------ +# Jan: +# > Sorry, I misread your question, attributes are like invisible for the tool, +# > they can be placed wherever between each of the QualifierOrder items. +# +# Hoho, one thing where various options are tolerated. +# +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02469.html +# ------------------------------------------------------------------------------ +# Luca: +# >> Hoho, one thing where various options are tolerated. +# +# I realise now that my answer above is not fully correct, I wanted to say that +# the attributes are like invisible for the tool only when dealing with this +# rule about ordering of the qualifiers. +# +# On the other hand, yes, if the QualifierAlignment is “Leave”, clang-format +# won’t touch them. +# +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02479.html +# ------------------------------------------------------------------------------ +# Jan: +# > On the other hand, yes, if the QualifierAlignment is “Leave”, clang-format +# > won’t touch them. +# +# Hmm - gets me back to wondering what "Alignment" in the name is about, when +# order of syntactical elements is affected. +# +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02481.html +# ------------------------------------------------------------------------------ +# Luca: +# Yes, probably QualifierAlignment is not the best name. +QualifierAlignment: Custom + +# Specify in which order the qualifiers should appear 'static const int *var' +# (supported in clang-format 14) +QualifierOrder: ['static', 'inline', 'const', 'volatile', 'type'] + +# Don't try to reflow comments to respect the column limit (It helps to make +# clang-format reproduce the same output when it runs on an already formatted +# file) +# (supported in clang-format 3.8) +ReflowComments: true + +# [not specified] +# Specifies the use of empty lines to separate definition blocks, including +# structs, enums, and functions +# (supported in clang-format 14) +SeparateDefinitionBlocks: Always + +# [not specified] +# Sort the include files by name (supported in clang-format 3.8) +# TODO: enabling this will currently break the compilation, maybe fix? +SortIncludes: Never + +# [not specified] +# Do not format macro definition body +# (supported in clang-format 3.18) +SkipMacroDefinitionBody: true + +# [not specified] +# Do not allow a space after a type cast '(type)var' +# (supported in clang-format 3.5) +SpaceAfterCStyleCast: false + +# [not specified] +# Do not allow a space after the not operator '!var' +# (supported in clang-format 9) +SpaceAfterLogicalNot: false + +# Ensure that there is a space after pointer qualifiers, when used with +# PointerAlignment of value Right it means 'void *const *x = NULL;' +# (supported in clang-format 12) +SpaceAroundPointerQualifiers: After + +# [not specified] +# Always have a space after an assign operator or a compound operator, for +# example '+=' (supported in clang-format 3.7) +SpaceBeforeAssignmentOperators: true + +# [not specified] +# Do not allow a space between the case argument and the final colon 'case 0:' +# (supported in clang-format 12) +SpaceBeforeCaseColon: false + +# [not specified] +# Have a space before the parenthesis of a control statement, do an exception +# for ForEach and If macros +SpaceBeforeParens: ControlStatementsExceptForEachMacros + +# (supported in clang-format 7) +SpaceBeforeRangeBasedForLoopColon: true + +# [not specified] +# Do not add a spaces before square brackets 'int a[5];' +# (supported in clang-format 10) +SpaceBeforeSquareBrackets: false + +# [not specified] +# Do not add a space inside empty parenthesis '()' +# (supported in clang-format 3.7) +SpaceInEmptyParentheses: false + +# (supported in clang-format 3.7) +SpacesBeforeTrailingComments: 1 + +# Space characters are used to spread out logical statements, such as in the +# condition of an if or while 'if ( a )' 'while ( i < 5 )' +# (supported in clang-format 10) +SpacesInConditionalStatement: true + +# (supported in clang-format 3.7) +SpacesInContainerLiterals: false + +# [not specified] +# Do not add a spaces inside a type cast parenthesis '(int)var' +# (supported in clang-format 3.7) +SpacesInCStyleCastParentheses: false + +# [not specified] +# Do not insert spaces in empty block '{}' +# (supported in clang-format 3.5) +SpaceInEmptyBlock: false + +# Only one space is allowed at the start of a line comment +# (supported in clang-format 13) +SpacesInLineCommentPrefix: + Minimum: 1 + Maximum: -1 + +# [not specified] +# Do not add a spaces inside parenthesis '(var & var)' +# (supported in clang-format 3.7) +SpacesInParentheses: false + +# [not specified] +# Do not add spaces inside square brakets 'int a[5];' +# (supported in clang-format 3.7) +SpacesInSquareBrackets: false + +# (supported in clang-format 3.7) +# +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg00833.html +# ------------------------------------------------------------------------------ +# Jan: Since I continue to be puzzled - iirc you said this is because of lack +# of availability of "C99" as a value here. What's entirely unclear to +# me is: How does this matter to a tool checking coding style (which is +# largely about formatting, not any lexical or syntactical aspects)? +# +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg00867.html +# ------------------------------------------------------------------------------ +# Luca: Ok, maybe I understand your point, you are looking for a reason to declare this +# configurable instead of not specifying it at all? +# If it’s that, from what I understand clang-format will use the default value if +# we don’t specify anything for this one, so it will take ‘Latest’. I think we +# should put a value for this one to fix it and don’t have surprises if that +# behaviour changes and seeing that also in Linux that value is fixed +# increased myconfidence. +# However, if you feel that we should not specify it, I’ve done a test and not +# specifying it is not changing the current output. I can’t say that for a +# different clang-format version though or if changes happen in the future. +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg00887.html +# ------------------------------------------------------------------------------ +# Alejandro: On C it _shouldn't_ matter because it's meant to affect C++ constructs +# only. That said, clang-format doesn't understand (or care) whether the code +# is C or C++, because C's syntax is strictly contained in that of C++ as far +# as formatting cares. +# While I agree it feels wrong to apply a C++ policy to a C project, it's +# largely irrelevant. Setting a value here gives more deterministic output +# because it fixes several options' default settings. One would hope none of +# those settings affect C, but the world is complex and it's better to be +# safe than sorry. Particularly when it's an inocuous one-liner. +# There aren't strict C values. And Latest or Auto are simply shorthands for +# one of the C++ options. +# +# https://clang.llvm.org/docs/ClangFormatStyleOptions.html#standard +# +# IMO, C++03 is as good as any other. As long as it's a fixed one. +# +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg00888.html +# ------------------------------------------------------------------------------ +# >>> This value is used also in Linux. +# >> +# >> Considering how different the two styles are, I don't think this is +# >> overly relevant. +# > +# > Ok, maybe I understand your point, you are looking for a reason to declare +# > this configurable instead of not specifying it at all? +# +# Jan: Not really, no. Here I was merely saying that with the styles being +# sufficiently different, what Linux uses is probably not very significant +# for our own decision. +# +# > However, if you feel that we should not specify it, I’ve done a test and not +# > specifying it is not changing +# > the current output. I can’t say that for a different clang-format version +# > though or if changes happen in the +# > future. +# Jan: It's fine to set values. All I'm saying is that at least I would prefer +# if it was also clear what exact effect the setting of a value has, +# especially when that does not really match the language we use in the +# project. +# +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg01035.html +# ------------------------------------------------------------------------------ +# Luca: Yes I agree, I think Alejandro’s reply to this configurable reflects my +# thoughts about it. +# So if we all agree that we should set this parameter, do we all agree that it +# should be the value above? +# Do you have other concerns regarding this or the other parameters in this +# thread? +# +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg01051.html +# ------------------------------------------------------------------------------ +# Alejandro: +# > > It's fine to set values. All I'm saying is that at least I would prefer +# > > if it was also clear what exact effect the setting of a value has, +# > > especially when that does not really match the language we use in the +# > > project. +# On C, allegedly, none. It ought to control defaults for things like +# SpaceBeforeCpp11BracedList, SpacesInAngles and other C++-specific things, +# because the C++ language sticks syntactical extensions every other Tuesday. +# Alas, whatever it does (there's no full list). I'd feel a lot more +# comfortable knowing it won't change under our feet. +# For reference, clang-format's docs state as an example: +# ``` +# c++03: latest: +# vector<set<int> > x; vs. vector<set<int>> x; +# ``` +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg01062.html +# ------------------------------------------------------------------------------ +# Jan: +# > Do you have other concerns regarding this or the other parameters in this +# > thread? +# I did raise what was occurring to me. This doesn't mean that down the +# road yet something else might not pop up. +# +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg01063.html +# ------------------------------------------------------------------------------ +# Luca: +# > I did raise what was occurring to me. This doesn't mean that down the +# > road yet something else might not pop up. +# Sure, thanks for raising your concern, I’ve asked so that at the deadline if no +# other concern are raised, we can move on with another set of configurable to +# discuss. +Standard: C++03 + +# Macros which are ignored in front of a statement, as if they were an +# attribute. So that they are not parsed as identifier +# (supported in clang-format 12) +# StatementAttributeLikeMacros: [''] + +# A vector of macros that should be interpreted as complete statements. +# Typical macros are expressions, and require a semi-colon to be added; +# sometimes this is not the case, and this allows to make clang-format aware of +# such cases. (supported in clang-format 8) +# +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg00833.html +# ------------------------------------------------------------------------------ +# Jan: Ah, just wanted to ask. I agree that we'd better have here only items +# which truly require an exception. +# +# ------------------------------------------------------------------------------ +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg00867.html +# ------------------------------------------------------------------------------ +# Luca: Yes my mistake, I’ll fix the list. +StatementMacros: + - 'PROGRESS' + - 'PROGRESS_VCPU' + - 'bitop' + - 'guest_bitop' + - 'testop' + - 'guest_testop' + +# An indent level consists of four spaces (supported in clang-format 3.7) +TabWidth: 4 + +# A vector of macros that should be interpreted as type declarations instead of +# as function calls (supported in clang-format 9) +TypenameMacros: + - 'XEN_GUEST_HANDLE' + - 'XEN_GUEST_HANDLE_64' + - 'XEN_GUEST_HANDLE_PARAM' + - 'ELF_HANDLE_DECL' + +# Indentation is done using spaces, not tabs (supported in clang-format 3.7) +UseTab: Never + +# A vector of macros which are whitespace-sensitive and should not be touched +# (supported in clang-format 11) +WhitespaceSensitiveMacros: + - '__stringify' + +## Not related to C language ################################################### + +# (supported in clang-format 3.3) +AccessModifierOffset: -4 + +# (supported in clang-format 9) +AllowShortLambdasOnASingleLine: None + +# (supported in clang-format 16) +# BreakAfterAttributes: Never + +# (supported in clang-format 3.8) +BreakAfterJavaFieldAnnotations: false + +# (supported in clang-format 16) +BreakArrays: true + +# (supported in clang-format 12) +BreakBeforeConceptDeclarations: Never + +# (supported in clang-format 7) +BreakInheritanceList: BeforeColon + +# (supported in clang-format 5) +CompactNamespaces: false + +# (supported in clang-format 3.7) +ConstructorInitializerAllOnOneLineOrOnePerLine: false + +# (supported in clang-format 3.7) +ConstructorInitializerIndentWidth: 4 + +# (supported in clang-format 3.4) +Cpp11BracedListStyle: false + +# (supported in clang-format 13) +EmptyLineAfterAccessModifier: Leave + +# (supported in clang-format 12) +EmptyLineBeforeAccessModifier: Leave + +# (supported in clang-format 5) +FixNamespaceComments: false + +# (supported in clang-format 13) +IndentAccessModifiers: false + +# (supported in clang-format 15) +IndentRequiresClause: false + +# (supported in clang-format 11) +InsertTrailingCommas: None + +# (supported in clang-format 3.9) +JavaScriptQuotes: Leave + +# (supported in clang-format 3.9) +JavaScriptWrapImports: true + +# (supported in clang-format 3.7) +NamespaceIndentation: None + +# (supported in clang-format 7) +ObjCBinPackProtocolList: Auto + +# (supported in clang-format 3.7) +ObjCBlockIndentWidth: 4 + +# (supported in clang-format 11) +ObjCBreakBeforeNestedBlockParam: false + +# (supported in clang-format 3.7) +ObjCSpaceAfterProperty: true + +# (supported in clang-format 3.7) +ObjCSpaceBeforeProtocolList: true + +# (supported in clang-format 14) +PackConstructorInitializers: Never + +# (supported in clang-format 7) +PenaltyBreakTemplateDeclaration: 0 + +# (supported in clang-format 13) +ReferenceAlignment: Right + +# (supported in clang-format 16) +# RemoveSemicolon: false + +# (supported in clang-format 5) +SortUsingDeclarations: false + +# (supported in clang-format 4) +SpaceAfterTemplateKeyword: true + +# (supported in clang-format 7) +SpaceBeforeCpp11BracedList: false + +# (supported in clang-format 7) +SpaceBeforeCtorInitializerColon: false + +# (supported in clang-format 7) +SpaceBeforeInheritanceColon: true + +# (supported in clang-format 3.4) +SpacesInAngles: false + +RemoveBracesLLVM: true + +... -- 2.25.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] Add .clang-format files to enable manual coding style checks 2025-03-01 11:42 ` [PATCH 1/4] Add .clang-format files to enable manual coding style checks Oleksandr Andrushchenko @ 2025-03-03 15:05 ` Roger Pau Monné 2025-03-03 19:23 ` Oleksandr Andrushchenko 0 siblings, 1 reply; 10+ messages in thread From: Roger Pau Monné @ 2025-03-03 15:05 UTC (permalink / raw) To: Oleksandr Andrushchenko Cc: xen-devel, sstabellini, Artem_Mygaiev, jbeulich, Luca.Fancellu, marmarek, andrew.cooper3, anthony.perard On Sat, Mar 01, 2025 at 01:42:39PM +0200, Oleksandr Andrushchenko wrote: > Disable coding style checks for the project, but xen/ folder: > this is done by providing a global .clang-format at the top level which > disables clang-format and only providing .clang-format for xen/. > > clang-format version expected to be >15 and the latest tool can be > installed with: > python3 -m pip install clang-format > > Please note, that no automatic code style checks are performed and 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. I think the comments need to be trimmed to a concise explanation of why a setting is the way it is. The (IMO overly long) excerpts from email conversations are difficult to follow in this context. Someone has to spend the time to digest them and provide a shorter comment. One thing that I'm missing: don't we need a list of excluded files somewhere because they are imports from Linux and we would like to keep the Linux coding style for avoiding conflicts when backporting fixes? > > Signed-off-by: Oleksandr Andrushchenko <andr2000@gmail.com> > Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> > --- > .clang-format | 5 + > xen/.clang-format | 1380 +++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 1385 insertions(+) > create mode 100644 .clang-format > create mode 100644 xen/.clang-format > > diff --git a/.clang-format b/.clang-format > new file mode 100644 > index 000000000000..fc3f8e167f65 > --- /dev/null > +++ b/.clang-format > @@ -0,0 +1,5 @@ > +--- > +DisableFormat: true > +SortIncludes: Never > +... > + > diff --git a/xen/.clang-format b/xen/.clang-format > new file mode 100644 > index 000000000000..480628544cdd > --- /dev/null > +++ b/xen/.clang-format > @@ -0,0 +1,1380 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +# > +# clang-format configuration file. Intended for clang-format >= 15. What happens if used with clang-format < 15? I expect the tool gives some kind of error about unknown options, rather than doing a wrong formatting? > +# > +# For more information, see: > +# > +# Documentation/process/clang-format.rst > +# https://clang.llvm.org/docs/ClangFormat.html > +# https://clang.llvm.org/docs/ClangFormatStyleOptions.html > +# > +--- > + > +################################################################################ > +# How to review the changes > +################################################################################ > +# ------------------------------------------------------------------------------ > +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02126.html > +# ------------------------------------------------------------------------------ > +# George Dunlap: > +# It's hard to agree on this one without seeing some of the examples of > +# what it does, some examples of the "weird behavior" Stefano & > +# Allejandro found, and some examples of places where it's going to > +# remove the alignment. > +# > +# I had tried to apply your series before and didn't get very far with > +# it for some reason ISTR. One way to see the effect of individual > +# features would be: > +# > +# 1. Make a branch with one big patch applying clang-format for a given style > +# > +# 2. Change just one style line, re-run clang format, and create a new > +# patch from that > +# > +# Then it would be easy to see the difference between the two. It might > +# actually be easier for individual reviewers to do that on their own > +# trees, rather than to ask you to try to generate and post such patches > +# somewhere. > + > +################################################################################ > +# do/while loops > +################################################################################ > +# ------------------------------------------------------------------------------ > +# https://lists.xenproject.org/archives/html/xen-devel/2023-07/msg02276.html > +# ------------------------------------------------------------------------------ > +# Jan: > +# > I've read the past threads about the brave people who dared to try to > +# > introduce clang-format for the xen codebase, some of them from 5 years ago, > +# > two points were clear: 1) goto label needs to be indented and 2) do-while > +# > loops have the braket in the same line. > +# > While point 1) was quite a blocker, it seemd to me that point 2) was less > +# > controversial to be changed in the Xen codestyle, so the current wrapper > +# > script handles only the point 1 (which is easy), the point 2 can be more > +# > tricky to handle. > +# > +# I'm afraid I view the do/while part pretty much as a blocker as well. > +# While placing the opening brace according to our style elsewhere would > +# be okay-ish (just a little wasteful to have two almost empty lines), > +# having the closing brace on a separate line is problematic: At least I > +# consider a block / scope to end at the line where the closing brace is. > +# So the farther do and while are apart, the more > +# > +# do > +# { > +# ...; > +# } > +# while ( cond ); > +# ...; > +# > +# is going to be misleading. While normally we would write potentially > +# conflicting constructs this way > +# > +# while ( cond ) > +# ; > +# > +# the alternative spelling still isn't outright wrong in our style (I > +# believe): > +# > +# while ( cond ); > +# > +# ------------------------------------------------------------------------------ > +# https://lists.xenproject.org/archives/html/xen-devel/2023-07/msg02282.html > +# ------------------------------------------------------------------------------ > +# Luca: > +# Thank you for your feedback, I could maybe misunderstood your reply, so please > +# tell me if I am wrong, the Xen coding style mandates this style for do-while > +# loops: > +# > +# do { > +# /* Do stuff. */ > +# } while ( condition ); > +# > +# Currently clang-format is able to do only this: > +# > +# do > +# { > +# /* Do stuff. */ > +# } while ( condition ); > +# > +# So the issue is only in the opening brackets, not the closing one. Is it a > +# blocker too? > +# > +# ------------------------------------------------------------------------------ > +# https://lists.xenproject.org/archives/html/xen-devel/2023-07/msg02284.html > +# ------------------------------------------------------------------------------ > +# Jan: > +# > do > +# > { > +# > /* Do stuff. */ > +# > } while ( condition ); > +# > +# Oh, I hadn't understood your description that way. > +# > +# > So the issue is only in the opening brackets, not the closing one. Is it a > +# > blocker too? > +# > +# No. I don't like the longer form, but I could live with it. > + > +################################################################################ > +# clang version to use > +################################################################################ > +# ------------------------------------------------------------------------------ > +# https://lists.xenproject.org/archives/html/xen-devel/2023-07/msg02301.html > +# ------------------------------------------------------------------------------ > +# Alejandro: > +# > The minimum clang-format version for the file is 15, my ubuntu 22.04 comes > +# > with it, we can reason if it's too high, or if we could also use the latest > +# > version maybe shipped inside a docker image. > +# 15 sounds ok \methinks. In practice we'll just stick it in GitLab and let > +# it check commits, so we'll be safe even in the case where every developer > +# has a slightly different version of the tool. I wouldn't try to make this > +# (hard) task harder by trying to retrofit it in an older version. > +# > +# It might be worth stitching a flag in the python script to scream if the > +#clang-format version doesn't match. Or it may do a mess of the tree. ^ missing space Is it possible to add such flag and give a clear error when the wrong clang-format version is used? > +################################################################################ > +# Options and their discussions > +################################################################################ > + > +# [not specified] > +# Align function parameter that goes into a new line, under the open bracket > +# (supported in clang-format 3.8) > +# ------------------------------------------------------------------------------ > +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg01993.html > +# ------------------------------------------------------------------------------ > +# Luca: This one is to align function parameters that overflows the line length, > +# I chose to align them to the open bracket to match the current codebase > +# (hopefully) e.g.: > +# someLongFunction(argument1, > +# argument2); > +# > +# This one can be a candidate for an entry in our coding style > +# ------------------------------------------------------------------------------ > +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02143.html > +# ------------------------------------------------------------------------------ > +# Jan: > +# > e.g.: > +# > someLongFunction(argument1, > +# > argument2); > +# The above matches neither of the two generally permitted styles: > +# > +# someLongFunction(argument1, > +# argument2); > +# > +# someLongFunction( > +# argument1, > +# argument2); > +# > +# Then again from its name I would infer this isn't just about function > +# arguments? > +# > +# ------------------------------------------------------------------------------ > +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02172.html > +# ------------------------------------------------------------------------------ > +# Luca: > +# I think it applies to parameters and arguments of functions and macro, given > +# the description in the docs. > +# I see your two snippets above but I’ve always found at least on arm a > +# predominance of the style above for functions, so arguments aligned after the > +# opening bracket, for macros there is a mix. > +# I might be wrong though and so another opinion from another maintainer would > +# help. > +# In any case we can choose among many value: > +# https://clang.llvm.org/docs/ClangFormatStyleOptions.html#alignafteropenbracket, > +# but when we do so, we need to stick to one permitted style only, the tool don’t > +# allow to specify more than one. > +# > +# ------------------------------------------------------------------------------ > +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02311.html > +# ------------------------------------------------------------------------------ > +# Jan: > +# > I see your two snippets above but I’ve always found at least on arm a > +# > predominance of the style above for functions, so arguments aligned after > +# > the opening bracket, for macros there is a mix. > +# > +# The latter "above" refers to which form exactly? The one you originally > +# spelled out, or the former of what my reply had? > +# > +# > In any case we can choose among many value: > +# > https://clang.llvm.org/docs/ClangFormatStyleOptions.html#alignafteropenbracket, > +# > but when we do so, we need to stick to one permitted style only, the tool > +# > don’t allow to specify more than one. > +# > +# a2k: RED FLAG What does 'a2k' mean? I'm afraid this needs to be trimmed to a more manageable comment, otherwise I simply get lost in the context. Maybe it makes sense more people involved in the discussion. Thanks, Roger. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] Add .clang-format files to enable manual coding style checks 2025-03-03 15:05 ` Roger Pau Monné @ 2025-03-03 19:23 ` Oleksandr Andrushchenko 2025-03-04 8:17 ` Roger Pau Monné 0 siblings, 1 reply; 10+ messages in thread From: Oleksandr Andrushchenko @ 2025-03-03 19:23 UTC (permalink / raw) To: Roger Pau Monné Cc: xen-devel, sstabellini, Artem_Mygaiev, jbeulich, Luca.Fancellu, marmarek, andrew.cooper3, anthony.perard Hello, Roger! On 03.03.25 17:05, Roger Pau Monné wrote: > On Sat, Mar 01, 2025 at 01:42:39PM +0200, Oleksandr Andrushchenko wrote: >> Disable coding style checks for the project, but xen/ folder: >> this is done by providing a global .clang-format at the top level which >> disables clang-format and only providing .clang-format for xen/. >> >> clang-format version expected to be >15 and the latest tool can be >> installed with: >> python3 -m pip install clang-format >> >> Please note, that no automatic code style checks are performed and 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. > I think the comments need to be trimmed to a concise explanation of > why a setting is the way it is. The (IMO overly long) excerpts from > email conversations are difficult to follow in this context. Someone > has to spend the time to digest them and provide a shorter comment. This is the goal. I do not expect the patch to be committed right now, so I left those intentionally to provide some background when those options were discussed before and, possibly, to address some questions which were brought earlier. The final patch will have all those in the way you mention here > > One thing that I'm missing: don't we need a list of excluded files > somewhere because they are imports from Linux and we would like to > keep the Linux coding style for avoiding conflicts when backporting > fixes? All files are excluded from formatting, but xen/ directory: this is by providing a top level .clang-format file which disables all formatting and also providing xen/.clang-format which is exactly for xen/. What is more, there is no automation yet, so this is up to a user to decide whether they want to run git clang-format or not. > >> Signed-off-by: Oleksandr Andrushchenko <andr2000@gmail.com> >> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> >> --- >> .clang-format | 5 + >> xen/.clang-format | 1380 +++++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 1385 insertions(+) >> create mode 100644 .clang-format >> create mode 100644 xen/.clang-format >> >> diff --git a/.clang-format b/.clang-format >> new file mode 100644 >> index 000000000000..fc3f8e167f65 >> --- /dev/null >> +++ b/.clang-format >> @@ -0,0 +1,5 @@ >> +--- >> +DisableFormat: true >> +SortIncludes: Never >> +... >> + >> diff --git a/xen/.clang-format b/xen/.clang-format >> new file mode 100644 >> index 000000000000..480628544cdd >> --- /dev/null >> +++ b/xen/.clang-format >> @@ -0,0 +1,1380 @@ >> +# SPDX-License-Identifier: GPL-2.0-only >> +# >> +# clang-format configuration file. Intended for clang-format >= 15. > What happens if used with clang-format < 15? I expect the tool gives > some kind of error about unknown options, rather than doing a wrong > formatting? Exactly, it will throw an error. > >> +# >> +# For more information, see: >> +# >> +# Documentation/process/clang-format.rst >> +# https://clang.llvm.org/docs/ClangFormat.html >> +# https://clang.llvm.org/docs/ClangFormatStyleOptions.html >> +# >> +--- >> + >> +################################################################################ >> +# How to review the changes >> +################################################################################ >> +# ------------------------------------------------------------------------------ >> +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02126.html >> +# ------------------------------------------------------------------------------ >> +# George Dunlap: >> +# It's hard to agree on this one without seeing some of the examples of >> +# what it does, some examples of the "weird behavior" Stefano & >> +# Allejandro found, and some examples of places where it's going to >> +# remove the alignment. >> +# >> +# I had tried to apply your series before and didn't get very far with >> +# it for some reason ISTR. One way to see the effect of individual >> +# features would be: >> +# >> +# 1. Make a branch with one big patch applying clang-format for a given style >> +# >> +# 2. Change just one style line, re-run clang format, and create a new >> +# patch from that >> +# >> +# Then it would be easy to see the difference between the two. It might >> +# actually be easier for individual reviewers to do that on their own >> +# trees, rather than to ask you to try to generate and post such patches >> +# somewhere. >> + >> +################################################################################ >> +# do/while loops >> +################################################################################ >> +# ------------------------------------------------------------------------------ >> +# https://lists.xenproject.org/archives/html/xen-devel/2023-07/msg02276.html >> +# ------------------------------------------------------------------------------ >> +# Jan: >> +# > I've read the past threads about the brave people who dared to try to >> +# > introduce clang-format for the xen codebase, some of them from 5 years ago, >> +# > two points were clear: 1) goto label needs to be indented and 2) do-while >> +# > loops have the braket in the same line. >> +# > While point 1) was quite a blocker, it seemd to me that point 2) was less >> +# > controversial to be changed in the Xen codestyle, so the current wrapper >> +# > script handles only the point 1 (which is easy), the point 2 can be more >> +# > tricky to handle. >> +# >> +# I'm afraid I view the do/while part pretty much as a blocker as well. >> +# While placing the opening brace according to our style elsewhere would >> +# be okay-ish (just a little wasteful to have two almost empty lines), >> +# having the closing brace on a separate line is problematic: At least I >> +# consider a block / scope to end at the line where the closing brace is. >> +# So the farther do and while are apart, the more >> +# >> +# do >> +# { >> +# ...; >> +# } >> +# while ( cond ); >> +# ...; >> +# >> +# is going to be misleading. While normally we would write potentially >> +# conflicting constructs this way >> +# >> +# while ( cond ) >> +# ; >> +# >> +# the alternative spelling still isn't outright wrong in our style (I >> +# believe): >> +# >> +# while ( cond ); >> +# >> +# ------------------------------------------------------------------------------ >> +# https://lists.xenproject.org/archives/html/xen-devel/2023-07/msg02282.html >> +# ------------------------------------------------------------------------------ >> +# Luca: >> +# Thank you for your feedback, I could maybe misunderstood your reply, so please >> +# tell me if I am wrong, the Xen coding style mandates this style for do-while >> +# loops: >> +# >> +# do { >> +# /* Do stuff. */ >> +# } while ( condition ); >> +# >> +# Currently clang-format is able to do only this: >> +# >> +# do >> +# { >> +# /* Do stuff. */ >> +# } while ( condition ); >> +# >> +# So the issue is only in the opening brackets, not the closing one. Is it a >> +# blocker too? >> +# >> +# ------------------------------------------------------------------------------ >> +# https://lists.xenproject.org/archives/html/xen-devel/2023-07/msg02284.html >> +# ------------------------------------------------------------------------------ >> +# Jan: >> +# > do >> +# > { >> +# > /* Do stuff. */ >> +# > } while ( condition ); >> +# >> +# Oh, I hadn't understood your description that way. >> +# >> +# > So the issue is only in the opening brackets, not the closing one. Is it a >> +# > blocker too? >> +# >> +# No. I don't like the longer form, but I could live with it. >> + >> +################################################################################ >> +# clang version to use >> +################################################################################ >> +# ------------------------------------------------------------------------------ >> +# https://lists.xenproject.org/archives/html/xen-devel/2023-07/msg02301.html >> +# ------------------------------------------------------------------------------ >> +# Alejandro: >> +# > The minimum clang-format version for the file is 15, my ubuntu 22.04 comes >> +# > with it, we can reason if it's too high, or if we could also use the latest >> +# > version maybe shipped inside a docker image. >> +# 15 sounds ok \methinks. In practice we'll just stick it in GitLab and let >> +# it check commits, so we'll be safe even in the case where every developer >> +# has a slightly different version of the tool. I wouldn't try to make this >> +# (hard) task harder by trying to retrofit it in an older version. >> +# >> +# It might be worth stitching a flag in the python script to scream if the >> +#clang-format version doesn't match. Or it may do a mess of the tree. > ^ missing space > > Is it possible to add such flag and give a clear error when the wrong > clang-format version is used? This is actually a question if clang-format of a given version supports options in .clang-format or not. If it doesn't it will throw an error anyways. > >> +################################################################################ >> +# Options and their discussions >> +################################################################################ >> + >> +# [not specified] >> +# Align function parameter that goes into a new line, under the open bracket >> +# (supported in clang-format 3.8) >> +# ------------------------------------------------------------------------------ >> +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg01993.html >> +# ------------------------------------------------------------------------------ >> +# Luca: This one is to align function parameters that overflows the line length, >> +# I chose to align them to the open bracket to match the current codebase >> +# (hopefully) e.g.: >> +# someLongFunction(argument1, >> +# argument2); >> +# >> +# This one can be a candidate for an entry in our coding style >> +# ------------------------------------------------------------------------------ >> +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02143.html >> +# ------------------------------------------------------------------------------ >> +# Jan: >> +# > e.g.: >> +# > someLongFunction(argument1, >> +# > argument2); >> +# The above matches neither of the two generally permitted styles: >> +# >> +# someLongFunction(argument1, >> +# argument2); >> +# >> +# someLongFunction( >> +# argument1, >> +# argument2); >> +# >> +# Then again from its name I would infer this isn't just about function >> +# arguments? >> +# >> +# ------------------------------------------------------------------------------ >> +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02172.html >> +# ------------------------------------------------------------------------------ >> +# Luca: >> +# I think it applies to parameters and arguments of functions and macro, given >> +# the description in the docs. >> +# I see your two snippets above but I’ve always found at least on arm a >> +# predominance of the style above for functions, so arguments aligned after the >> +# opening bracket, for macros there is a mix. >> +# I might be wrong though and so another opinion from another maintainer would >> +# help. >> +# In any case we can choose among many value: >> +# https://clang.llvm.org/docs/ClangFormatStyleOptions.html#alignafteropenbracket, >> +# but when we do so, we need to stick to one permitted style only, the tool don’t >> +# allow to specify more than one. >> +# >> +# ------------------------------------------------------------------------------ >> +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02311.html >> +# ------------------------------------------------------------------------------ >> +# Jan: >> +# > I see your two snippets above but I’ve always found at least on arm a >> +# > predominance of the style above for functions, so arguments aligned after >> +# > the opening bracket, for macros there is a mix. >> +# >> +# The latter "above" refers to which form exactly? The one you originally >> +# spelled out, or the former of what my reply had? >> +# >> +# > In any case we can choose among many value: >> +# > https://clang.llvm.org/docs/ClangFormatStyleOptions.html#alignafteropenbracket, >> +# > but when we do so, we need to stick to one permitted style only, the tool >> +# > don’t allow to specify more than one. >> +# >> +# a2k: RED FLAG > What does 'a2k' mean? > > I'm afraid this needs to be trimmed to a more manageable comment, > otherwise I simply get lost in the context. Maybe it makes sense more > people involved in the discussion. Sure, see my explanation above: this is just to give some more context and have some ready answers for the questions being asked before. I will remove all these right after we have some consensus. > > Thanks, Roger. Thank you, Oleksandr ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] Add .clang-format files to enable manual coding style checks 2025-03-03 19:23 ` Oleksandr Andrushchenko @ 2025-03-04 8:17 ` Roger Pau Monné 2025-03-04 20:04 ` Oleksandr Andrushchenko 0 siblings, 1 reply; 10+ messages in thread From: Roger Pau Monné @ 2025-03-04 8:17 UTC (permalink / raw) To: Oleksandr Andrushchenko Cc: xen-devel, sstabellini, Artem_Mygaiev, jbeulich, Luca.Fancellu, marmarek, andrew.cooper3, anthony.perard On Mon, Mar 03, 2025 at 09:23:57PM +0200, Oleksandr Andrushchenko wrote: > Hello, Roger! > > On 03.03.25 17:05, Roger Pau Monné wrote: > > On Sat, Mar 01, 2025 at 01:42:39PM +0200, Oleksandr Andrushchenko wrote: > > > Disable coding style checks for the project, but xen/ folder: > > > this is done by providing a global .clang-format at the top level which > > > disables clang-format and only providing .clang-format for xen/. > > > > > > clang-format version expected to be >15 and the latest tool can be > > > installed with: > > > python3 -m pip install clang-format > > > > > > Please note, that no automatic code style checks are performed and 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. > > I think the comments need to be trimmed to a concise explanation of > > why a setting is the way it is. The (IMO overly long) excerpts from > > email conversations are difficult to follow in this context. Someone > > has to spend the time to digest them and provide a shorter comment. > This is the goal. I do not expect the patch to be committed right > now, so I left those intentionally to provide some background > when those options were discussed before and, possibly, to > address some questions which were brought earlier. > The final patch will have all those in the way you mention here > > > > One thing that I'm missing: don't we need a list of excluded files > > somewhere because they are imports from Linux and we would like to > > keep the Linux coding style for avoiding conflicts when backporting > > fixes? > All files are excluded from formatting, but xen/ directory: this is > by providing a top level .clang-format file which disables all > formatting and also providing xen/.clang-format which is exactly > for xen/. There are files with intentional Linux coding style inside of xen/, for example xen/arch/x86/cpu/mwait-idle.c. We would need a way to filter those out so that clang-format doesn't attempt to apply the Xen coding style to them. Otherwise it needs to be noted possibly in .clang-format that while the clang-format style will apply by default to all files inside of xen/, some of those use a different coding style on purpose, and those should not be reformatted. We also have some scripts in python and shell inside of both tools/ and scripts/, are those already excluded by clang-format by not having a .c or .h extension? > > > +################################################################################ > > > +# Options and their discussions > > > +################################################################################ > > > + > > > +# [not specified] > > > +# Align function parameter that goes into a new line, under the open bracket > > > +# (supported in clang-format 3.8) > > > +# ------------------------------------------------------------------------------ > > > +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg01993.html > > > +# ------------------------------------------------------------------------------ > > > +# Luca: This one is to align function parameters that overflows the line length, > > > +# I chose to align them to the open bracket to match the current codebase > > > +# (hopefully) e.g.: > > > +# someLongFunction(argument1, > > > +# argument2); > > > +# > > > +# This one can be a candidate for an entry in our coding style > > > +# ------------------------------------------------------------------------------ > > > +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02143.html > > > +# ------------------------------------------------------------------------------ > > > +# Jan: > > > +# > e.g.: > > > +# > someLongFunction(argument1, > > > +# > argument2); > > > +# The above matches neither of the two generally permitted styles: > > > +# > > > +# someLongFunction(argument1, > > > +# argument2); > > > +# > > > +# someLongFunction( > > > +# argument1, > > > +# argument2); > > > +# > > > +# Then again from its name I would infer this isn't just about function > > > +# arguments? > > > +# > > > +# ------------------------------------------------------------------------------ > > > +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02172.html > > > +# ------------------------------------------------------------------------------ > > > +# Luca: > > > +# I think it applies to parameters and arguments of functions and macro, given > > > +# the description in the docs. > > > +# I see your two snippets above but I’ve always found at least on arm a > > > +# predominance of the style above for functions, so arguments aligned after the > > > +# opening bracket, for macros there is a mix. > > > +# I might be wrong though and so another opinion from another maintainer would > > > +# help. > > > +# In any case we can choose among many value: > > > +# https://clang.llvm.org/docs/ClangFormatStyleOptions.html#alignafteropenbracket, > > > +# but when we do so, we need to stick to one permitted style only, the tool don’t > > > +# allow to specify more than one. > > > +# > > > +# ------------------------------------------------------------------------------ > > > +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02311.html > > > +# ------------------------------------------------------------------------------ > > > +# Jan: > > > +# > I see your two snippets above but I’ve always found at least on arm a > > > +# > predominance of the style above for functions, so arguments aligned after > > > +# > the opening bracket, for macros there is a mix. > > > +# > > > +# The latter "above" refers to which form exactly? The one you originally > > > +# spelled out, or the former of what my reply had? > > > +# > > > +# > In any case we can choose among many value: > > > +# > https://clang.llvm.org/docs/ClangFormatStyleOptions.html#alignafteropenbracket, > > > +# > but when we do so, we need to stick to one permitted style only, the tool > > > +# > don’t allow to specify more than one. > > > +# > > > +# a2k: RED FLAG > > What does 'a2k' mean? > > > > I'm afraid this needs to be trimmed to a more manageable comment, > > otherwise I simply get lost in the context. Maybe it makes sense more > > people involved in the discussion. > Sure, see my explanation above: this is just to give some more context > and have some ready answers for the questions being asked before. > I will remove all these right after we have some consensus. Sorry for possibly not fully understanding the purpose and being stubborn, but IMO consensus should be reached on the email discussion, so that here you can make a proposal, possibly with a comment justifying the chosen value. Just copying excerpts from the email thread is not likely to help, as I at least find myself completely lost. Otherwise if there's no consensus just add a TODO next to the option explaining why the current setting might not be optimal or the (known) shortcomings of it, so that we know there's some pending work in this area. But it needs to be a concrete list, not pieces from an email discussion. Thanks, Roger. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] Add .clang-format files to enable manual coding style checks 2025-03-04 8:17 ` Roger Pau Monné @ 2025-03-04 20:04 ` Oleksandr Andrushchenko 0 siblings, 0 replies; 10+ messages in thread From: Oleksandr Andrushchenko @ 2025-03-04 20:04 UTC (permalink / raw) To: Roger Pau Monné Cc: xen-devel, sstabellini, Artem_Mygaiev, jbeulich, Luca.Fancellu, marmarek, andrew.cooper3, anthony.perard Hello, Roger! On 04.03.25 10:17, Roger Pau Monné wrote: > On Mon, Mar 03, 2025 at 09:23:57PM +0200, Oleksandr Andrushchenko wrote: >> Hello, Roger! >> >> On 03.03.25 17:05, Roger Pau Monné wrote: >>> On Sat, Mar 01, 2025 at 01:42:39PM +0200, Oleksandr Andrushchenko wrote: >>>> Disable coding style checks for the project, but xen/ folder: >>>> this is done by providing a global .clang-format at the top level which >>>> disables clang-format and only providing .clang-format for xen/. >>>> >>>> clang-format version expected to be >15 and the latest tool can be >>>> installed with: >>>> python3 -m pip install clang-format >>>> >>>> Please note, that no automatic code style checks are performed and 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. >>> I think the comments need to be trimmed to a concise explanation of >>> why a setting is the way it is. The (IMO overly long) excerpts from >>> email conversations are difficult to follow in this context. Someone >>> has to spend the time to digest them and provide a shorter comment. >> This is the goal. I do not expect the patch to be committed right >> now, so I left those intentionally to provide some background >> when those options were discussed before and, possibly, to >> address some questions which were brought earlier. >> The final patch will have all those in the way you mention here >>> One thing that I'm missing: don't we need a list of excluded files >>> somewhere because they are imports from Linux and we would like to >>> keep the Linux coding style for avoiding conflicts when backporting >>> fixes? >> All files are excluded from formatting, but xen/ directory: this is >> by providing a top level .clang-format file which disables all >> formatting and also providing xen/.clang-format which is exactly >> for xen/. > There are files with intentional Linux coding style inside of xen/, > for example xen/arch/x86/cpu/mwait-idle.c. We would need a way to > filter those out so that clang-format doesn't attempt to apply the Xen > coding style to them. > > Otherwise it needs to be noted possibly in .clang-format that while > the clang-format style will apply by default to all files inside of > xen/, some of those use a different coding style on purpose, and those > should not be reformatted. There is an analog of .gitignore which is .clang-format-ignore [1] > > We also have some scripts in python and shell inside of both tools/ > and scripts/, are those already excluded by clang-format by not having > a .c or .h extension? Currently it is only C/C++ files with the settings in the .clang-format provided > >>>> +################################################################################ >>>> +# Options and their discussions >>>> +################################################################################ >>>> + >>>> +# [not specified] >>>> +# Align function parameter that goes into a new line, under the open bracket >>>> +# (supported in clang-format 3.8) >>>> +# ------------------------------------------------------------------------------ >>>> +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg01993.html >>>> +# ------------------------------------------------------------------------------ >>>> +# Luca: This one is to align function parameters that overflows the line length, >>>> +# I chose to align them to the open bracket to match the current codebase >>>> +# (hopefully) e.g.: >>>> +# someLongFunction(argument1, >>>> +# argument2); >>>> +# >>>> +# This one can be a candidate for an entry in our coding style >>>> +# ------------------------------------------------------------------------------ >>>> +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02143.html >>>> +# ------------------------------------------------------------------------------ >>>> +# Jan: >>>> +# > e.g.: >>>> +# > someLongFunction(argument1, >>>> +# > argument2); >>>> +# The above matches neither of the two generally permitted styles: >>>> +# >>>> +# someLongFunction(argument1, >>>> +# argument2); >>>> +# >>>> +# someLongFunction( >>>> +# argument1, >>>> +# argument2); >>>> +# >>>> +# Then again from its name I would infer this isn't just about function >>>> +# arguments? >>>> +# >>>> +# ------------------------------------------------------------------------------ >>>> +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02172.html >>>> +# ------------------------------------------------------------------------------ >>>> +# Luca: >>>> +# I think it applies to parameters and arguments of functions and macro, given >>>> +# the description in the docs. >>>> +# I see your two snippets above but I’ve always found at least on arm a >>>> +# predominance of the style above for functions, so arguments aligned after the >>>> +# opening bracket, for macros there is a mix. >>>> +# I might be wrong though and so another opinion from another maintainer would >>>> +# help. >>>> +# In any case we can choose among many value: >>>> +# https://clang.llvm.org/docs/ClangFormatStyleOptions.html#alignafteropenbracket, >>>> +# but when we do so, we need to stick to one permitted style only, the tool don’t >>>> +# allow to specify more than one. >>>> +# >>>> +# ------------------------------------------------------------------------------ >>>> +# https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg02311.html >>>> +# ------------------------------------------------------------------------------ >>>> +# Jan: >>>> +# > I see your two snippets above but I’ve always found at least on arm a >>>> +# > predominance of the style above for functions, so arguments aligned after >>>> +# > the opening bracket, for macros there is a mix. >>>> +# >>>> +# The latter "above" refers to which form exactly? The one you originally >>>> +# spelled out, or the former of what my reply had? >>>> +# >>>> +# > In any case we can choose among many value: >>>> +# > https://clang.llvm.org/docs/ClangFormatStyleOptions.html#alignafteropenbracket, >>>> +# > but when we do so, we need to stick to one permitted style only, the tool >>>> +# > don’t allow to specify more than one. >>>> +# >>>> +# a2k: RED FLAG >>> What does 'a2k' mean? >>> >>> I'm afraid this needs to be trimmed to a more manageable comment, >>> otherwise I simply get lost in the context. Maybe it makes sense more >>> people involved in the discussion. >> Sure, see my explanation above: this is just to give some more context >> and have some ready answers for the questions being asked before. >> I will remove all these right after we have some consensus. > Sorry for possibly not fully understanding the purpose and being > stubborn, but IMO consensus should be reached on the email discussion, > so that here you can make a proposal, possibly with a comment > justifying the chosen value. Just copying excerpts from the email > thread is not likely to help, as I at least find myself completely > lost. Agree, but the opposite happened before: same questions asked again as we are out of context each time... > > Otherwise if there's no consensus just add a TODO next to the option > explaining why the current setting might not be optimal or the (known) > shortcomings of it, so that we know there's some pending work in this > area. But it needs to be a concrete list, not pieces from an email > discussion. It is not that easy as options act as a set, we cannot have one option per rule unfortunately. > > Thanks, Roger. Thank you, Oleksandr [1] https://clang.llvm.org/docs/ClangFormat.html#clang-format-ignore ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/4] [EXAMPLE] Provide an example of a malformed patch 2025-03-01 11:42 [PATCH 0/4] clang-format with examples Oleksandr Andrushchenko 2025-03-01 11:42 ` [PATCH 1/4] Add .clang-format files to enable manual coding style checks Oleksandr Andrushchenko @ 2025-03-01 11:42 ` Oleksandr Andrushchenko 2025-03-01 11:42 ` [PATCH 3/4] [EXAMPLE] Changes done by clang-format to the previous commit Oleksandr Andrushchenko ` (2 subsequent siblings) 4 siblings, 0 replies; 10+ messages in thread From: Oleksandr Andrushchenko @ 2025-03-01 11:42 UTC (permalink / raw) To: xen-devel Cc: andr2000, sstabellini, Artem_Mygaiev, jbeulich, Luca.Fancellu, roger.pau, marmarek, andrew.cooper3, anthony.perard This patch touches tools/xl and xen/. clang-format, if applied, will only change xen/ part. Signed-off-by: Oleksandr Andrushchenko <andr2000@gmail.com> --- tools/xl/xl_info.c | 6 ++---- xen/drivers/char/ns16550.c | 12 +++++------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/tools/xl/xl_info.c b/tools/xl/xl_info.c index 72e87eac46d1..cbb0f695685f 100644 --- a/tools/xl/xl_info.c +++ b/tools/xl/xl_info.c @@ -59,8 +59,7 @@ static int maybe_printf(const char *fmt, ...) return count; } -static yajl_gen_status printf_info_one_json(yajl_gen hand, int domid, - libxl_domain_config *d_config) +static yajl_gen_status printf_info_one_json(yajl_gen hand, int domid, libxl_domain_config *d_config) { yajl_gen_status s; @@ -68,8 +67,7 @@ static yajl_gen_status printf_info_one_json(yajl_gen hand, int domid, if (s != yajl_gen_status_ok) goto out; - s = yajl_gen_string(hand, (const unsigned char *)"domid", - sizeof("domid")-1); + s = yajl_gen_string(hand, (const unsigned char *)"domid", sizeof("domid")-1); if (s != yajl_gen_status_ok) goto out; if (domid != -1) diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c index eaeb0e09d01e..53586f110008 100644 --- a/xen/drivers/char/ns16550.c +++ b/xen/drivers/char/ns16550.c @@ -340,9 +340,7 @@ static void ns16550_setup_preirq(struct ns16550 *uart) if ( divisor ) uart->baud = uart->clock_hz / (divisor << 4); else - printk(XENLOG_ERR - "Automatic baud rate determination was requested," - " but a baud rate was not set up\n"); + printk(XENLOG_ERR "Automatic baud rate determination was requested," " but a baud rate was not set up\n"); } ns_write_reg(uart, UART_LCR, lcr); @@ -350,8 +348,8 @@ static void ns16550_setup_preirq(struct ns16550 *uart) ns_write_reg(uart, UART_MCR, UART_MCR_DTR | UART_MCR_RTS); /* Enable and clear the FIFOs. Set a large trigger threshold. */ - ns_write_reg(uart, UART_FCR, - UART_FCR_ENABLE | UART_FCR_CLRX | UART_FCR_CLTX | UART_FCR_TRG14); + ns_write_reg(uart, UART_FCR, UART_FCR_ENABLE | UART_FCR_CLRX | + UART_FCR_CLTX | UART_FCR_TRG14); } static void __init cf_check ns16550_init_preirq(struct serial_port *port) @@ -398,8 +396,8 @@ static void ns16550_setup_postirq(struct ns16550 *uart) if ( uart->irq > 0 ) { /* Master interrupt enable; also keep DTR/RTS asserted. */ - ns_write_reg(uart, - UART_MCR, UART_MCR_OUT2 | UART_MCR_DTR | UART_MCR_RTS); + ns_write_reg(uart, UART_MCR, + UART_MCR_OUT2 | UART_MCR_DTR | UART_MCR_RTS); /* Enable receive interrupts. */ ns_write_reg(uart, UART_IER, UART_IER_ERDAI); -- 2.25.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/4] [EXAMPLE] Changes done by clang-format to the previous commit 2025-03-01 11:42 [PATCH 0/4] clang-format with examples Oleksandr Andrushchenko 2025-03-01 11:42 ` [PATCH 1/4] Add .clang-format files to enable manual coding style checks Oleksandr Andrushchenko 2025-03-01 11:42 ` [PATCH 2/4] [EXAMPLE] Provide an example of a malformed patch Oleksandr Andrushchenko @ 2025-03-01 11:42 ` Oleksandr Andrushchenko 2025-03-01 11:42 ` [PATCH 4/4] [EXAMPLE] Whole file code style formatting Oleksandr Andrushchenko 2025-03-04 19:09 ` [PATCH 0/4] clang-format with examples Stefano Stabellini 4 siblings, 0 replies; 10+ messages in thread From: Oleksandr Andrushchenko @ 2025-03-01 11:42 UTC (permalink / raw) To: xen-devel Cc: andr2000, sstabellini, Artem_Mygaiev, jbeulich, Luca.Fancellu, roger.pau, marmarek, andrew.cooper3, anthony.perard This patch is the result of the: $ git-clang-format --style=file --verbose HEAD~1 Running clang-format on the following files: tools/xl/xl_info.c xen/drivers/char/ns16550.c old tree: dceb97b3ef3c80cbb9c29311a7f5783df2f69127 new tree: a8d36f8f45c3ee0d75e674ecda102ffc6a0a0ef3 changed files: xen/drivers/char/ns16550.c It is seen that only xen/ part of the patch was touched. The result can still be manually adjusted before committing. Signed-off-by: Oleksandr Andrushchenko <andr2000@gmail.com> --- xen/drivers/char/ns16550.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c index 53586f110008..4d79ef2b08bc 100644 --- a/xen/drivers/char/ns16550.c +++ b/xen/drivers/char/ns16550.c @@ -340,7 +340,9 @@ static void ns16550_setup_preirq(struct ns16550 *uart) if ( divisor ) uart->baud = uart->clock_hz / (divisor << 4); else - printk(XENLOG_ERR "Automatic baud rate determination was requested," " but a baud rate was not set up\n"); + printk( + XENLOG_ERR + "Automatic baud rate determination was requested," " but a baud rate was not set up\n"); } ns_write_reg(uart, UART_LCR, lcr); @@ -348,8 +350,9 @@ static void ns16550_setup_preirq(struct ns16550 *uart) ns_write_reg(uart, UART_MCR, UART_MCR_DTR | UART_MCR_RTS); /* Enable and clear the FIFOs. Set a large trigger threshold. */ - ns_write_reg(uart, UART_FCR, UART_FCR_ENABLE | UART_FCR_CLRX | - UART_FCR_CLTX | UART_FCR_TRG14); + ns_write_reg(uart, UART_FCR, + UART_FCR_ENABLE | UART_FCR_CLRX | UART_FCR_CLTX | + UART_FCR_TRG14); } static void __init cf_check ns16550_init_preirq(struct serial_port *port) -- 2.25.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 4/4] [EXAMPLE] Whole file code style formatting 2025-03-01 11:42 [PATCH 0/4] clang-format with examples Oleksandr Andrushchenko ` (2 preceding siblings ...) 2025-03-01 11:42 ` [PATCH 3/4] [EXAMPLE] Changes done by clang-format to the previous commit Oleksandr Andrushchenko @ 2025-03-01 11:42 ` Oleksandr Andrushchenko 2025-03-04 19:09 ` [PATCH 0/4] clang-format with examples Stefano Stabellini 4 siblings, 0 replies; 10+ messages in thread From: Oleksandr Andrushchenko @ 2025-03-01 11:42 UTC (permalink / raw) To: xen-devel Cc: andr2000, sstabellini, Artem_Mygaiev, jbeulich, Luca.Fancellu, roger.pau, marmarek, andrew.cooper3, anthony.perard This patch was produced by running clang-format on a file: clang-format -i xen/drivers/pci/pci.c Signed-off-by: Oleksandr Andrushchenko <andr2000@gmail.com> --- xen/drivers/pci/pci.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/xen/drivers/pci/pci.c b/xen/drivers/pci/pci.c index edf5b9f7ae9f..f6d8222c1dd4 100644 --- a/xen/drivers/pci/pci.c +++ b/xen/drivers/pci/pci.c @@ -56,10 +56,8 @@ unsigned int pci_find_next_cap_ttl(pci_sbdf_t sbdf, unsigned int pos, if ( id == 0xff ) break; for ( i = 0; i < n; i++ ) - { if ( id == caps[i] ) return pos; - } pos = (pos & ~3) + PCI_CAP_LIST_NEXT; } @@ -116,7 +114,8 @@ unsigned int pci_find_next_ext_capability(pci_sbdf_t sbdf, unsigned int start, return 0; ASSERT(start != pos || PCI_EXT_CAP_ID(header) == cap); - while ( ttl-- > 0 ) { + while ( ttl-- > 0 ) + { if ( PCI_EXT_CAP_ID(header) == cap && pos != start ) return pos; pos = PCI_EXT_CAP_NEXT(header); @@ -174,8 +173,7 @@ const char *__init parse_pci_seg(const char *s, unsigned int *seg_p, } else func = 0; - if ( seg != (seg_p ? (u16)seg : 0) || - bus != PCI_BUS(PCI_BDF(bus, 0)) || + if ( seg != (seg_p ? (u16)seg : 0) || bus != PCI_BUS(PCI_BDF(bus, 0)) || dev != PCI_SLOT(PCI_DEVFN(dev, 0)) || func != PCI_FUNC(PCI_DEVFN(0, func)) ) return NULL; -- 2.25.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 0/4] clang-format with examples 2025-03-01 11:42 [PATCH 0/4] clang-format with examples Oleksandr Andrushchenko ` (3 preceding siblings ...) 2025-03-01 11:42 ` [PATCH 4/4] [EXAMPLE] Whole file code style formatting Oleksandr Andrushchenko @ 2025-03-04 19:09 ` Stefano Stabellini 4 siblings, 0 replies; 10+ messages in thread From: Stefano Stabellini @ 2025-03-04 19:09 UTC (permalink / raw) To: Oleksandr Andrushchenko Cc: xen-devel, sstabellini, Artem_Mygaiev, jbeulich, Luca.Fancellu, roger.pau, marmarek, andrew.cooper3, anthony.perard 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 > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-03-04 20:05 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-03-01 11:42 [PATCH 0/4] clang-format with examples Oleksandr Andrushchenko 2025-03-01 11:42 ` [PATCH 1/4] Add .clang-format files to enable manual coding style checks Oleksandr Andrushchenko 2025-03-03 15:05 ` Roger Pau Monné 2025-03-03 19:23 ` Oleksandr Andrushchenko 2025-03-04 8:17 ` Roger Pau Monné 2025-03-04 20:04 ` Oleksandr Andrushchenko 2025-03-01 11:42 ` [PATCH 2/4] [EXAMPLE] Provide an example of a malformed patch Oleksandr Andrushchenko 2025-03-01 11:42 ` [PATCH 3/4] [EXAMPLE] Changes done by clang-format to the previous commit Oleksandr Andrushchenko 2025-03-01 11:42 ` [PATCH 4/4] [EXAMPLE] Whole file code style formatting Oleksandr Andrushchenko 2025-03-04 19:09 ` [PATCH 0/4] clang-format with examples Stefano Stabellini
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.