Merge pull request #5482 from pks-t/pks/coding-style docs: add documentation for our coding style
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 215 216 217 218 219 220 221 222 223 224 225 226 227 228 229 230 231 232 233 234 235 236 237 238 239 240 241 242 243 244 245 246 247 248 249 250 251 252 253 254 255 256 257 258 259 260 261 262 263 264 265 266 267 268 269 270 271 272 273 274 275 276 277 278 279 280 281 282 283 284 285 286 287 288 289 290 291 292 293 294 295 296 297 298 299 300 301 302 303 304 305 306 307 308 309 310 311 312 313 314 315 316 317 318 319 320 321 322 323 324 325 326 327 328 329 330 331 332 333 334 335 336 337 338 339 340 341 342 343 344 345 346 347 348 349 350 351 352 353 354 355 356 357 358 359 360 361 362 363 364 365 366 367 368 369 370
diff --git a/docs/coding-style.md b/docs/coding-style.md
new file mode 100644
index 0000000..d5188f0
--- /dev/null
+++ b/docs/coding-style.md
@@ -0,0 +1,364 @@
+# libgit2 Coding Style
+
+This documentation describes the preferred coding style for the libgit2 project.
+While not all parts of our code base conform to this coding style, the outlined
+rules are what we aim for.
+
+Note that in no case do we accept changes that convert huge parts of the code
+base to use our coding style. Instead, it is encouraged to modernize small parts
+of code you're going to modify anyway for a given change you want to introduce.
+A good rule to follow is the Boy Scout Rule: "Leave the campground cleaner than
+you found it."
+
+## C Coding Style
+
+The following sections define the coding style for all code files and headers.
+
+### Indentation and Alignment
+
+Code is indented by tabs, where a tab is 8 spaces. Each opening scope increases
+the indentation level.
+
+```c
+int foobar(int void)
+{
+ if (condition)
+ doit();
+ /* Body */
+}
+```
+
+Switch statements have their `case`s aligned with the `switch` keyword. Case
+bodies are indented by an additional level. Case bodies should not open their
+own scope to declare variables.
+
+```c
+switch (c) {
+case 'a':
+case 'b':
+ return 0;
+default:
+ return -1;
+}
+```
+
+Multi-line conditions should be aligned with the opening brace of the current
+statement:
+
+```c
+if (one_very_long_condition(c) &&
+ another_very_long_condition(c))
+ doit();
+```
+
+### Spaces
+
+There must be no space between the function and its arguments, arguments must be
+separated by a space:
+
+```c
+int doit(int first_arg, int second_arg);
+doit(1, 2);
+```
+
+For any binary or ternary operators, the arguments and separator must be
+separated by a space:
+
+```c
+1 + 2;
+x ? x : NULL;
+```
+
+Unary operators do not have a space between them and the argument they refer to:
+
+```c
+*c
+&c
+```
+
+The `sizeof` operator always must not have a space and must use braces around
+the type:
+
+```
+sizeof(int)
+```
+
+There must be a space after the keywords `if`, `switch`, `case`, `do` and
+`while`.
+
+### Braces
+
+Functions must have their opening brace on the following line:
+
+```c
+void foobar(void)
+{
+ doit();
+}
+```
+
+For conditions, braces should be placed on the same line as the condition:
+
+```c
+if (condition(c)) {
+ doit();
+ dothat();
+}
+
+while (true) {
+ doit();
+}
+```
+
+In case a condition's body has a single line, only, it's allowed to omit braces,
+except if any of its `else if` or `else` branches has more than one line:
+
+```c
+if (condition(c))
+ doit();
+
+if (condition(c))
+ doit();
+else if (other_condition(c))
+ doit();
+
+/* This example must use braces as the `else if` requires them. */
+if (condition(c)) {
+ doit();
+} else if (other_condition(c)) {
+ doit();
+ dothat();
+} else {
+ abort();
+}
+```
+
+### Comments
+
+Comments must use C-style `/* */` comments. C++-style `// `comments are not
+allowed in our codebase. This is a strict requirement as libgit2 tries to be
+compliant with the ISO C90 standard, which only allows C-style comments.
+
+Single-line comments may have their opening and closing tag on the same line:
+
+```c
+/* This is a short comment. */
+```
+
+For multi-line comments, the opening and closing tag should be empty:
+
+```c
+/*
+ * This is a rather long and potentially really unwiedly but informative
+ * multiline comment that helps quite a lot.
+ */
+```
+
+Public functions must have documentation that explain their usage, internal
+functions should have a comment. We use Docurium to generate documentation
+derived from these comments, which uses syntax similar to Doxygen. The first
+line should be a short summary of what the function does. More in-depth
+explanation should be separated from that first line by an empty line.
+Parameters and return values should be documented via `@return` and `@param`
+tags:
+
+```c
+/*
+ * Froznicate the string.
+ *
+ * Froznicate the string by foobaring its internal structure into a more obvious
+ * translation. Note that the returned string is a newly allocated string that
+ * shall be `free`d by the caller.
+ *
+ * @param s String to froznicate
+ * @return A newly allocated string or `NULL` in case an error occurred.
+ * /
+char *froznicate(const char *s);
+```
+
+### Variables
+
+Variables must be declared at the beginning of their scope. This is a strict
+requirement as libgit2 tries to be compliant with the ISO C90 standard, which
+forbids mixed declarations and code:
+
+```c
+void foobar(void)
+{
+ char *c = NULL;
+ int a, b;
+
+ a = 0;
+ b = 1;
+
+ return c;
+}
+```
+
+### Naming
+
+Variables must have all-lowercase names. In case a variable name has multiple
+words, words should be separated by an underscore `_` character. While
+recommended to use descriptive naming, common variable names like `i` for
+indices are allowed.
+
+All public functions must have a `git` prefix as well as a prefix indicating
+their respective subsystem. E.g. a function that opens a repository should be
+called `git_repository_open()`. Functions that are not public but declared in
+an internal header file for use by other subsystems should follow the same
+naming pattern. File-local static functions must not have a `git` prefix, but
+should have a prefix indicating their respective subsystem.
+
+All structures declared in the libgit2 project must have a `typedef`, we do not
+use `struct type` variables. Type names follow the same schema as functions.
+
+### Error Handling
+
+The libgit2 project mostly uses error codes to indicate errors. Error codes are
+always of type `int`, where `0` indicates success and a negative error code
+indicates an error case. In some cases, positive error codes may be used to
+indicate special cases. Returned values that are not an error code should be
+returned via an out parameter. Out parameters must always come first in the list
+of arguments.
+
+```c
+int doit(const char **out, int arg)
+{
+ if (!arg)
+ return -1;
+ *out = "Got an argument";
+ return 0;
+}
+```
+
+To avoid repetitive and fragile error handling in case a function has resources
+that need to be free'd, we use `goto out`s:
+
+```c
+int doit(char **out, int arg)
+{
+ int error = 0;
+ char *c;
+
+ c = malloc(strlen("Got an argument") + 1);
+ if (!c) {
+ error = -1;
+ goto out;
+ }
+
+ if (!arg) {
+ error = -1;
+ goto out;
+ }
+
+ strcpy(c, "Got an argument")
+ *out = c;
+
+out:
+ if (error)
+ free(c);
+ return error;
+}
+```
+
+When calling functions that return an error code, you should assign the error
+code to an `error` variable and, in case an error case is indicated and no
+custom error handling is required, return that error code:
+
+```c
+int foobar(void)
+{
+ int error;
+
+ if ((error = doit()) < 0)
+ return error;
+
+ return 0;
+}
+```
+
+When doing multiple function calls where all of the functions return an error
+code, it's common practice to chain these calls together:
+
+```c
+int doit(void)
+{
+ int error;
+
+ if ((error = dothis()) < 0 ||
+ (error = dothat()) < 0)
+ return error;
+
+ return 0;
+}
+```
+
+## CMake Coding Style
+
+The following section defines the coding style for our CMake build system.
+
+### Indentation
+
+Code is indented by tabs, where a tab is 8 spaces. Each opening scope increases
+the indentation level.
+
+```cmake
+if(CONDITION)
+ doit()
+endif()
+```
+
+### Spaces
+
+There must be no space between keywords and their opening brace. While this is
+the same as in our C codebase for function calls, this also applies to
+conditional keywords. This is done to avoid the awkward-looking `else ()`
+statement.
+
+```cmake
+if(CONDITION)
+ doit()
+else()
+ dothat()
+endif()
+```
+
+### Case
+
+While CMake is completely case-insensitive when it comes to function calls, we
+want to agree on a common coding style for this. To reduce the danger of
+repetitive strain injuries, all function calls should be lower-case (NB: this is
+not currently the case yet, but introduced as a new coding style by this
+document).
+
+Variables are written all-uppercase. In contrast to functions, variables are
+case-sensitive in CMake. As CMake itself uses upper-case variables in all
+places, we should follow suit and do the same.
+
+Control flow keywords must be all lowercase. In contrast to that, test keywords
+must be all uppercase:
+
+```cmake
+if(NOT CONDITION)
+ doit()
+elseif(FOO AND BAR)
+ dothat()
+endif()
+```
+
+### Targets
+
+CMake code should not use functions that modify the global scope but prefer
+their targeted equivalents, instead. E.g. instead of using
+`include_directories()`, you must use `target_include_directories()`. An
+exception to this rule is setting up global compiler flags like warnings or
+flags required to set up the build type.
+
+### Dependencies
+
+Dependencies should not be discovered or set up in the main "CMakeLists.txt"
+module. Instead, they should either have their own module in our top-level
+"cmake/" directory or have a "CMakeLists.txt" in their respective "deps/"
+directory in case it is a vendored library. All dependencies should expose
+interface library targets that can be linked against with
+`target_link_libraries()`.