From 395726717bcc4073d2adba4ee5016a350dfa8d3a Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 30 Jul 2024 09:24:33 +0200 Subject: [PATCH 1/5] clang-format: fix indentation width for preprocessor directives In [1], we have improved our clang-format configuration to also specify the style for how to indent preprocessor directives. But while we have settled the question of where to put the indentation, either before or after the hash sign, we didn't specify exactly how to indent. With the current configuration, clang-format uses tabs to indent each level of nested preprocessor directives, which is in fact unintentional and never done in our codebase. Instead, we use a mixture of indenting by either one or two spaces, where using a single space is somewhat more common. Adapt our clang-format configuration accordingly by specifying an indentation width of one space. [1]: <20240708092317.267915-1-karthik.188@gmail.com> Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- .clang-format | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.clang-format b/.clang-format index 16fd12253e..0b82f3c776 100644 --- a/.clang-format +++ b/.clang-format @@ -100,11 +100,13 @@ BreakStringLiterals: false # Switch statement body is always indented one level more than case labels. IndentCaseLabels: false -# Indents directives before the hash. +# Indents directives before the hash. Each level uses a single space for +# indentation. # #if FOO -# # include +# # include # #endif IndentPPDirectives: AfterHash +PPIndentWidth: 1 # Don't indent a function definition or declaration if it is wrapped after the # type From 7df3f55b92ef39239b7b84fee6b89a87a304a58f Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 30 Jul 2024 09:24:38 +0200 Subject: [PATCH 2/5] Documentation: clarify indentation style for C preprocessor directives In the preceding commit, we have settled on using a single space per nesting level to indent preprocessor directives. Clarify our coding guidelines accordingly. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- Documentation/CodingGuidelines | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines index 1d92b2da03..65fba3b810 100644 --- a/Documentation/CodingGuidelines +++ b/Documentation/CodingGuidelines @@ -214,6 +214,16 @@ For C programs: - We use tabs to indent, and interpret tabs as taking up to 8 spaces. + - Nested C preprocessor directives are indented after the hash by one + space per nesting level. + + #if FOO + # include + # if BAR + # include + # endif + #endif + - We try to keep to at most 80 characters per line. - As a Git developer we assume you have a reasonably modern compiler From 541204aabea80ce466b5f62bf6613cdaf104dd5a Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 30 Jul 2024 09:24:43 +0200 Subject: [PATCH 3/5] Documentation: document naming schema for structs and their functions We nowadays have a proper mishmash of struct-related functions that are called `_` (e.g. `clear_prio_queue()`) versus functions that are called `_` (e.g. `strbuf_clear()`). While the former style may be easier to tie into a spoken conversation, most of our communication happens in text anyway. Furthermore, prefixing functions with the name of the structure they operate on makes it way easier to group them together, see which functions are related, and will also help folks who are using code completion. Let's thus settle on one style, namely the one where functions start with the name of the structure they operate on. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- Documentation/CodingGuidelines | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines index 65fba3b810..a6a1ede204 100644 --- a/Documentation/CodingGuidelines +++ b/Documentation/CodingGuidelines @@ -541,6 +541,25 @@ For C programs: use your own debugger and arguments. Example: `GIT_DEBUGGER="ddd --gdb" ./bin-wrappers/git log` (See `wrap-for-bin.sh`.) + - The primary data structure that a subsystem 'S' deals with is called + `struct S`. Functions that operate on `struct S` are named + `S_()` and should generally receive a pointer to `struct S` as + first parameter. E.g. + + struct strbuf; + + void strbuf_add(struct strbuf *buf, ...); + + void strbuf_reset(struct strbuf *buf); + + is preferred over: + + struct strbuf; + + void add_string(struct strbuf *buf, ...); + + void reset_strbuf(struct strbuf *buf); + For Perl programs: - Most of the C guidelines above apply. From 10f0723c8df41c9c2a4dec7e3571e98ec57138f1 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 30 Jul 2024 09:24:47 +0200 Subject: [PATCH 4/5] Documentation: document idiomatic function names We semi-regularly have discussions around whether a function shall be named `S_release()`, `S_clear()` or `S_free()`. Indeed, it may not be obvious which of these is preferable as we never really defined what each of these variants means exactly. Carve out a space where we can add idiomatic names for common functions in our coding guidelines and define each of those functions. Like this, we can get to a shared understanding of their respective semantics and can easily point towards our style guide in future discussions such that our codebase becomes more consistent over time. Note that the intent is not to rename all functions which violate these semantics right away. Rather, the intent is to slowly converge towards a common style over time. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- Documentation/CodingGuidelines | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines index a6a1ede204..b63a8f9a44 100644 --- a/Documentation/CodingGuidelines +++ b/Documentation/CodingGuidelines @@ -560,6 +560,23 @@ For C programs: void reset_strbuf(struct strbuf *buf); + - There are several common idiomatic names for functions performing + specific tasks on a structure `S`: + + - `S_init()` initializes a structure without allocating the + structure itself. + + - `S_release()` releases a structure's contents without freeing the + structure. + + - `S_clear()` is equivalent to `S_release()` followed by `S_init()` + such that the structure is directly usable after clearing it. When + `S_clear()` is provided, `S_init()` shall not allocate resources + that need to be released again. + + - `S_free()` releases a structure's contents and frees the + structure. + For Perl programs: - Most of the C guidelines above apply. From 6cda5972837360bf6a55884f3db45902afb0590d Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 30 Jul 2024 09:24:52 +0200 Subject: [PATCH 5/5] Documentation: consistently use spaces inside initializers Our coding guide is inconsistent with how it uses spaces inside of initializers (`struct foo bar = { something }`). While we mostly carry the space between open and closing braces and the initialized members, in one case we don't. Fix this one instance such that we consistently carry the space. This is also consistent with how clang-format formats such initializers. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- Documentation/CodingGuidelines | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines index b63a8f9a44..3daa1f82bb 100644 --- a/Documentation/CodingGuidelines +++ b/Documentation/CodingGuidelines @@ -244,7 +244,7 @@ For C programs: . since around 2007 with 2b6854c863a, we have been using initializer elements which are not computable at load time. E.g.: - const char *args[] = {"constant", variable, NULL}; + const char *args[] = { "constant", variable, NULL }; . since early 2012 with e1327023ea, we have been using an enum definition whose last element is followed by a comma. This, like