SELinux Security Module development
 help / color / mirror / Atom feed
From: Petr Lautrbach <lautrbach@redhat.com>
To: Stephen Smalley <stephen.smalley.work@gmail.com>,
	Rahul Sandhu <nvraxn@gmail.com>
Cc: selinux@vger.kernel.org
Subject: Re: [RFC PATCH v2] treewide: add .clang-format configuration file
Date: Tue, 07 Oct 2025 08:35:15 +0200	[thread overview]
Message-ID: <87ldln43to.fsf@redhat.com> (raw)
In-Reply-To: <CAEjxPJ5fK0HtMzn7kHsddvzC+Ku+BnK1YCe4ZJ-DG0_7WfSNwg@mail.gmail.com>

Stephen Smalley <stephen.smalley.work@gmail.com> writes:

> On Sun, Sep 28, 2025 at 6:48 PM Rahul Sandhu <nvraxn@gmail.com> wrote:
>>
>> Currently only an RFC.
>>
>> Add the .clang-format configuration file, taken from the Linux kernel
>> repository. We don't have any official style guidelines in tree at
>> present, which makes it a bit unclear how to format C code for new
>> contributors. As well as this, different parts of the codebase seem to
>> been formatted with different styles on occasion, so using an automatic
>> formatter should resolve this.
>>
>> .clang-format is also read by various editors and tooling for writing C
>> code. It may also be worth adding an editorconfig file in the future as
>> well?
>
> Can't hurt.
>
>> It may well also be worth adding a `format` target to the Makefiles, is
>> that something that would be useful?
>
> IMHO, yes.
>
>>
>> A few other things to consider to do in the future:
>> 1. Reformat all existing code. I understand this is a big change, hence
>>    the RFC, but we may as well get all code formatted if we go down
>>    this route; afterall, it's not like this will cause any breaking
>>    changes.
>
> Not opposed but will defer to the distro package maintainers on when
> they want to apply such a change since it makes back-porting future
> patches more painful for them.


As a Fedora and RHEL maintainer, this might be annoying in the short
term, but I see it as an improvement in the long term. 

If it's well-documented, with configuration files and a helper script
for formatting/checking available, and CI is set up, I would not be
opposed to this. 

>> 2. Possibly add a CI target to check that all code is formatted as per
>>    the new clang-format configuration? The options `--dry-run` as well
>>    as `-Werror` can be passed to clang-format for this.
>
> Sounds reasonable too.
>
>>
>> Comments/feedback appreciated, thanks.
>
> No strong opinions; would be fine with any standardized coding style.
> For comparison, selinux-testsuite has tools/check-syntax which can be
> used to check or fix coding style problems for C and perl code, but
> I'd be fine with using clang-format here as in the kernel.
>
>> Signed-off-by: Rahul Sandhu <nvraxn@gmail.com>
>> ---
>>  .clang-format   | 130 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  CONTRIBUTING.md |   2 +
>>  2 files changed, 132 insertions(+)
>>  create mode 100644 .clang-format
>>
>> v2: remove linux kernel ForEachMacros and replace them with ours
>>
>> diff --git a/.clang-format b/.clang-format
>> new file mode 100644
>> index 00000000..35595d87
>> --- /dev/null
>> +++ b/.clang-format
>> @@ -0,0 +1,130 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +#
>> +# clang-format configuration file. Intended for clang-format >= 11.
>> +#
>> +# For more information, see:
>> +#
>> +#   Documentation/dev-tools/clang-format.rst
>> +#   https://clang.llvm.org/docs/ClangFormat.html
>> +#   https://clang.llvm.org/docs/ClangFormatStyleOptions.html
>> +#
>> +---
>> +AccessModifierOffset: -4
>> +AlignAfterOpenBracket: Align
>> +AlignConsecutiveAssignments: false
>> +AlignConsecutiveDeclarations: false
>> +AlignEscapedNewlines: Left
>> +AlignOperands: true
>> +AlignTrailingComments: false
>> +AllowAllParametersOfDeclarationOnNextLine: false
>> +AllowShortBlocksOnASingleLine: false
>> +AllowShortCaseLabelsOnASingleLine: false
>> +AllowShortFunctionsOnASingleLine: None
>> +AllowShortIfStatementsOnASingleLine: false
>> +AllowShortLoopsOnASingleLine: false
>> +AlwaysBreakAfterDefinitionReturnType: None
>> +AlwaysBreakAfterReturnType: None
>> +AlwaysBreakBeforeMultilineStrings: false
>> +AlwaysBreakTemplateDeclarations: false
>> +BinPackArguments: true
>> +BinPackParameters: true
>> +BraceWrapping:
>> +  AfterClass: false
>> +  AfterControlStatement: false
>> +  AfterEnum: false
>> +  AfterFunction: true
>> +  AfterNamespace: true
>> +  AfterObjCDeclaration: false
>> +  AfterStruct: false
>> +  AfterUnion: false
>> +  AfterExternBlock: false
>> +  BeforeCatch: false
>> +  BeforeElse: false
>> +  IndentBraces: false
>> +  SplitEmptyFunction: true
>> +  SplitEmptyRecord: true
>> +  SplitEmptyNamespace: true
>> +BreakBeforeBinaryOperators: None
>> +BreakBeforeBraces: Custom
>> +BreakBeforeInheritanceComma: false
>> +BreakBeforeTernaryOperators: false
>> +BreakConstructorInitializersBeforeComma: false
>> +BreakConstructorInitializers: BeforeComma
>> +BreakAfterJavaFieldAnnotations: false
>> +BreakStringLiterals: false
>> +ColumnLimit: 80
>> +CommentPragmas: '^ IWYU pragma:'
>> +CompactNamespaces: false
>> +ConstructorInitializerAllOnOneLineOrOnePerLine: false
>> +ConstructorInitializerIndentWidth: 8
>> +ContinuationIndentWidth: 8
>> +Cpp11BracedListStyle: false
>> +DerivePointerAlignment: false
>> +DisableFormat: false
>> +ExperimentalAutoDetectBinPacking: false
>> +FixNamespaceComments: false
>> +
>> +# Taken from:
>> +#   git grep -h '^#define [^[:space:]]*for_each[^[:space:]]*(' \
>> +#   | sed "s,^#define \([^[:space:]]*for_each[^[:space:]]*\)(.*$,  - '\1'," \
>> +#   | LC_ALL=C sort -u
>> +ForEachMacros:
>> +  - 'cil_list_for_each'
>> +  - 'cil_stack_for_each'
>> +  - 'cil_stack_for_each_starting_at'
>> +  - 'ebitmap_for_each_bit'
>> +  - 'ebitmap_for_each_positive_bit'
>> +
>> +IncludeBlocks: Preserve
>> +IncludeCategories:
>> +  - Regex: '.*'
>> +    Priority: 1
>> +IncludeIsMainRegex: '(Test)?$'
>> +IndentCaseLabels: false
>> +IndentGotoLabels: false
>> +IndentPPDirectives: None
>> +IndentWidth: 8
>> +IndentWrappedFunctionNames: false
>> +JavaScriptQuotes: Leave
>> +JavaScriptWrapImports: true
>> +KeepEmptyLinesAtTheStartOfBlocks: false
>> +MacroBlockBegin: ''
>> +MacroBlockEnd: ''
>> +MaxEmptyLinesToKeep: 1
>> +NamespaceIndentation: None
>> +ObjCBinPackProtocolList: Auto
>> +ObjCBlockIndentWidth: 8
>> +ObjCSpaceAfterProperty: true
>> +ObjCSpaceBeforeProtocolList: true
>> +
>> +# Taken from git's rules
>> +PenaltyBreakAssignment: 10
>> +PenaltyBreakBeforeFirstCallParameter: 30
>> +PenaltyBreakComment: 10
>> +PenaltyBreakFirstLessLess: 0
>> +PenaltyBreakString: 10
>> +PenaltyExcessCharacter: 100
>> +PenaltyReturnTypeOnItsOwnLine: 60
>> +
>> +PointerAlignment: Right
>> +ReflowComments: false
>> +SortIncludes: false
>> +SortUsingDeclarations: false
>> +SpaceAfterCStyleCast: false
>> +SpaceAfterTemplateKeyword: true
>> +SpaceBeforeAssignmentOperators: true
>> +SpaceBeforeCtorInitializerColon: true
>> +SpaceBeforeInheritanceColon: true
>> +SpaceBeforeParens: ControlStatementsExceptForEachMacros
>> +SpaceBeforeRangeBasedForLoopColon: true
>> +SpaceInEmptyParentheses: false
>> +SpacesBeforeTrailingComments: 1
>> +SpacesInAngles: false
>> +SpacesInContainerLiterals: false
>> +SpacesInCStyleCastParentheses: false
>> +SpacesInParentheses: false
>> +SpacesInSquareBrackets: false
>> +Standard: Cpp03
>> +TabWidth: 8
>> +UseTab: Always
>> +...
>> diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
>> index c501cf84..7ec8cb0f 100644
>> --- a/CONTRIBUTING.md
>> +++ b/CONTRIBUTING.md
>> @@ -53,6 +53,8 @@ When preparing patches, please follow these guidelines:
>>  -   Separate large patches into logical patches
>>  -   Patch descriptions must end with your "Signed-off-by" line. This means your
>>      code meets the Developer's certificate of origin, see below.
>> +-   C code should be formatted using clang-format, using the .clang-format
>> +    configuration file at the root of this repository.
>>
>>  When adding new, large features or tools it is best to discuss the
>>  design on the mailing list prior to submitting the patch.
>> --
>> 2.50.1
>>
>>


  reply	other threads:[~2025-10-07  6:35 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-28 22:39 [RFC PATCH] treewide: add .clang-format configuration file Rahul Sandhu
2025-09-28 22:48 ` [RFC PATCH v2] " Rahul Sandhu
2025-10-06 16:52   ` Stephen Smalley
2025-10-07  6:35     ` Petr Lautrbach [this message]
2025-10-07 10:46       ` [PATCH v3] " Rahul Sandhu
2025-10-07 11:07         ` Rahul Sandhu
2025-10-07 13:24           ` Stephen Smalley
2025-10-07 13:21         ` Stephen Smalley
2025-10-16 22:05           ` Rahul Sandhu
2025-10-17 12:51             ` Stephen Smalley
2025-10-18  6:11               ` [RFC PATCH v4] " Rahul Sandhu
2025-10-20 13:23                 ` Stephen Smalley
2025-10-20 13:49                   ` Stephen Smalley
2025-10-20 14:07                     ` [PATCH v5] " Rahul Sandhu
2025-10-20 14:09                       ` [PATCH v6] " Rahul Sandhu
2025-10-20 14:55                         ` Stephen Smalley
2025-10-20 16:18                           ` When to apply `make format` to the entire tree Petr Lautrbach
2025-10-20 16:26                             ` Rahul Sandhu
2025-10-20 16:40                               ` Stephen Smalley
2025-10-21 13:49                           ` [PATCH v6] treewide: add .clang-format configuration file Stephen Smalley
2025-10-16 22:08           ` [PATCH v3] " Rahul Sandhu
2025-10-08 15:17     ` [RFC PATCH v2] " Kenton Groombridge

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87ldln43to.fsf@redhat.com \
    --to=lautrbach@redhat.com \
    --cc=nvraxn@gmail.com \
    --cc=selinux@vger.kernel.org \
    --cc=stephen.smalley.work@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox