Re: [PATCH v3] treewide: add .clang-format configuration file

["Rahul Sandhu" <nvraxn@gmail.com>]

On Tue Oct 7, 2025 at 2:21 PM BST, Stephen Smalley wrote:
> On Tue, Oct 7, 2025 at 6:47 AM 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.
>>
>> As well as this, replace all the existing indent targets with format
>> targets. Commands used to find and replace those targets:
>>
>> git grep -l -E '(\.\./)*scripts/Lindent' | xargs sed -i -E 's@(\.\./)*scripts/Lindent@clang-format -i@g'
>> git grep -l 'indent' -- '*Makefile' | xargs sed -i 's/indent/format/g'
>>
>> Also add some empty format targets to Makefiles that previously were
>> missing an indent target so that `make format` does not error.
>>
>> 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.
>> 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.
>
> Yes please.

Should this be added in another patch? My concern is that CI will fail
should this be merged without formatting all code.

>
>>
>> Comments/feedback appreciated, thanks.
>>
>> Signed-off-by: Rahul Sandhu <nvraxn@gmail.com>
>> ---
>>
>> v2: remove linux kernel ForEachMacros and replace them with ours
>> v3: replace the indent target with the new format target. also remove any
>>     mention of `.editorconfig` from the commit message; those changes are
>>     better suited for another patch (they're small and self-contained enough
>>     that they're a smaller thing to merge).
>>
>>
>
>> diff --git a/mcstrans/Makefile b/mcstrans/Makefile
>> index b20279ab..28d8c7bc 100644
>> --- a/mcstrans/Makefile
>> +++ b/mcstrans/Makefile
>> @@ -21,4 +21,6 @@ clean:
>>
>>  relabel:
>>
>> +format:
>> +
>
> We should add targets for mcstrans/utils/*.c and mcstrans/src/*.[hc]

I wrote a simple check-format target in the Makefile:

# We shouldn't have any unformatted files in the repo without an explicit exception.
# Given that this is used with the `check-format` target, which does not modify any
# source files, merely checking if they're formatted properly, it's fine to rely on
# find to get all C source and header files.
CHECK_FORMAT_SOURCE_FILES := $(shell find $(SUBDIRS) -type f \( -name '*.c' -o -name '*.h' \))

check-format:
	clang-format --dry-run -Werror $(CHECK_FORMAT_SOURCE_FILES)

However, after running `make format` and `make check-format`, I noticed
a whole load of failures caused by a load of other missing targets. I'm
thinking that it may just be better off using that same find command to
get all c sources and headers, and have a single, global format target.
Afterall, in pushed code after we've formatted it all, I don't see any
reason that some code should be left unformatted (especially as we have
things such as the clang-format off comments for specific sub-sections
of code), and while working locally if someone wishes to format only a
specific globbed set of files, something like this should work fine:
clang-format -i src/*.c

Thoughts? Suggestions?

>
>> diff --git a/secilc/Makefile b/secilc/Makefile
>> index ef7bc8cd..2518933f 100644
>> --- a/secilc/Makefile
>> +++ b/secilc/Makefile
>> @@ -87,4 +87,6 @@ clean:
>>
>>  relabel:
>>
>> +format:
>> +
>
> Should add a target for secilc/*.c