1--- 2title: Coding Style 3category: Contributing 4layout: default 5SPDX-License-Identifier: LGPL-2.1-or-later 6--- 7 8# Coding Style 9 10## Formatting 11 12- 8ch indent, no tabs, except for files in `man/` which are 2ch indent, and 13 still no tabs, and shell scripts, which are 4ch indent, and no tabs either. 14 15- We prefer `/* comments */` over `// comments` in code you commit, 16 please. This way `// comments` are left for developers to use for local, 17 temporary commenting of code for debug purposes (i.e. uncommittable stuff), 18 making such comments easily discernible from explanatory, documenting code 19 comments (i.e. committable stuff). 20 21- Don't break code lines too eagerly. We do **not** force line breaks at 80ch, 22 all of today's screens should be much larger than that. But then again, don't 23 overdo it, ~109ch should be enough really. The `.editorconfig`, `.vimrc` and 24 `.dir-locals.el` files contained in the repository will set this limit up for 25 you automatically, if you let them (as well as a few other things). Please 26 note that emacs loads `.dir-locals.el` automatically, but vim needs to be 27 configured to load `.vimrc`, see that file for instructions. 28 29- If you break a function declaration over multiple lines, do it like this: 30 31 ```c 32 void some_function( 33 int foo, 34 bool bar, 35 char baz) { 36 37 int a, b, c; 38 ``` 39 40 (i.e. use double indentation — 16 spaces — for the parameter list.) 41 42- Try to write this: 43 44 ```c 45 void foo() { 46 } 47 ``` 48 49 instead of this: 50 51 ```c 52 void foo() 53 { 54 } 55 ``` 56 57- Single-line `if` blocks should not be enclosed in `{}`. Write this: 58 59 ```c 60 if (foobar) 61 waldo(); 62 ``` 63 64 instead of this: 65 66 ```c 67 if (foobar) { 68 waldo(); 69 } 70 ``` 71 72- Do not write `foo ()`, write `foo()`. 73- `else` blocks should generally start on the same line as the closing `}`: 74 ```c 75 if (foobar) { 76 find(); 77 waldo(); 78 } else 79 dont_find_waldo(); 80 ``` 81 82- Please define flags types like this: 83 84 ```c 85 typedef enum FoobarFlags { 86 FOOBAR_QUUX = 1 << 0, 87 FOOBAR_WALDO = 1 << 1, 88 FOOBAR_XOXO = 1 << 2, 89 … 90 } FoobarFlags; 91 ``` 92 93 i.e. use an enum for it, if possible. Indicate bit values via `1 <<` 94 expressions, and align them vertically. Define both an enum and a type for 95 it. 96 97- If you define (non-flags) enums, follow this template: 98 99 ```c 100 typedef enum FoobarMode { 101 FOOBAR_AAA, 102 FOOBAR_BBB, 103 FOOBAR_CCC, 104 … 105 _FOOBAR_MAX, 106 _FOOBAR_INVALID = -EINVAL, 107 } FoobarMode; 108 ``` 109 110 i.e. define a `_MAX` enum for the largest defined enum value, plus one. Since 111 this is not a regular enum value, prefix it with `_`. Also, define a special 112 "invalid" enum value, and set it to `-EINVAL`. That way the enum type can 113 safely be used to propagate conversion errors. 114 115- If you define an enum in a public API, be extra careful, as the size of the 116 enum might change when new values are added, which would break ABI 117 compatibility. Since we typically want to allow adding new enum values to an 118 existing enum type with later API versions, please use the 119 `_SD_ENUM_FORCE_S64()` macro in the enum definition, which forces the size of 120 the enum to be signed 64bit wide. 121 122## Code Organization and Semantics 123 124- For our codebase we intend to use ISO C11 *with* GNU extensions (aka 125 "gnu11"). Public APIs (i.e. those we expose via `libsystemd.so` 126 i.e. `systemd/sd-*.h`) should only use ISO C89 however (with a very limited 127 set of conservative and common extensions, such as fixed size integer types 128 from `<inttypes.h>`), so that we don't force consuming programs into C11 129 mode. (This discrepancy in particular means one thing: internally we use C99 130 `bool` booleans, externally C89-compatible `int` booleans which generally 131 have different size in memory and slightly different semantics, also see 132 below.) Both for internal and external code it's OK to use even newer 133 features and GCC extension than "gnu11", as long as there's reasonable 134 fallback #ifdeffery in place to ensure compatibility is retained with older 135 compilers. 136 137- Please name structures in `PascalCase` (with exceptions, such as public API 138 structs), variables and functions in `snake_case`. 139 140- Avoid static variables, except for caches and very few other cases. Think 141 about thread-safety! While most of our code is never used in threaded 142 environments, at least the library code should make sure it works correctly 143 in them. Instead of doing a lot of locking for that, we tend to prefer using 144 TLS to do per-thread caching (which only works for small, fixed-size cache 145 objects), or we disable caching for any thread that is not the main 146 thread. Use `is_main_thread()` to detect whether the calling thread is the 147 main thread. 148 149- Do not write functions that clobber call-by-reference variables on 150 failure. Use temporary variables for these cases and change the passed in 151 variables only on success. The rule is: never clobber return parameters on 152 failure, always initialize return parameters on success. 153 154- Typically, function parameters fit into three categories: input parameters, 155 mutable objects, and call-by-reference return parameters. Input parameters 156 should always carry suitable "const" declarators if they are pointers, to 157 indicate they are input-only and not changed by the function. Return 158 parameters are best prefixed with "ret_", to clarify they are return 159 parameters. (Conversely, please do not prefix parameters that aren't 160 output-only with "ret_", in particular not mutable parameters that are both 161 input as well as output). Example: 162 163 ```c 164 static int foobar_frobnicate( 165 Foobar* object, /* the associated mutable object */ 166 const char *input, /* immutable input parameter */ 167 char **ret_frobnicated) { /* return parameter */ 168 … 169 return 0; 170 } 171 ``` 172 173- The order in which header files are included doesn't matter too 174 much. systemd-internal headers must not rely on an include order, so it is 175 safe to include them in any order possible. However, to not clutter global 176 includes, and to make sure internal definitions will not affect global 177 headers, please always include the headers of external components first 178 (these are all headers enclosed in <>), followed by our own exported headers 179 (usually everything that's prefixed by `sd-`), and then followed by internal 180 headers. Furthermore, in all three groups, order all includes alphabetically 181 so duplicate includes can easily be detected. 182 183- Please avoid using global variables as much as you can. And if you do use 184 them make sure they are static at least, instead of exported. Especially in 185 library-like code it is important to avoid global variables. Why are global 186 variables bad? They usually hinder generic reusability of code (since they 187 break in threaded programs, and usually would require locking there), and as 188 the code using them has side-effects make programs non-transparent. That 189 said, there are many cases where they explicitly make a lot of sense, and are 190 OK to use. For example, the log level and target in `log.c` is stored in a 191 global variable, and that's OK and probably expected by most. Also in many 192 cases we cache data in global variables. If you add more caches like this, 193 please be careful however, and think about threading. Only use static 194 variables if you are sure that thread-safety doesn't matter in your 195 case. Alternatively, consider using TLS, which is pretty easy to use with 196 gcc's `thread_local` concept. It's also OK to store data that is inherently 197 global in global variables, for example data parsed from command lines, see 198 below. 199 200- Our focus is on the GNU libc (glibc), not any other libcs. If other libcs are 201 incompatible with glibc it's on them. However, if there are equivalent POSIX 202 and Linux/GNU-specific APIs, we generally prefer the POSIX APIs. If there 203 aren't, we are happy to use GNU or Linux APIs, and expect non-GNU 204 implementations of libc to catch up with glibc. 205 206## Using C Constructs 207 208- Allocate local variables where it makes sense: at the top of the block, or at 209 the point where they can be initialized. Avoid huge variable declaration 210 lists at the top of the function. 211 212 As an exception, `r` is typically used for a local state variable, but should 213 almost always be declared as the last variable at the top of the function. 214 215 ```c 216 { 217 uint64_t a; 218 int r; 219 220 r = frobnicate(&a); 221 if (r < 0) 222 … 223 224 uint64_t b = a + 1, c; 225 226 r = foobarify(a, b, &c); 227 if (r < 0) 228 … 229 230 const char *pretty = prettify(a, b, c); 231 … 232 } 233 ``` 234 235- Do not mix multiple variable definitions with function invocations or 236 complicated expressions: 237 238 ```c 239 { 240 uint64_t x = 7; 241 int a; 242 243 a = foobar(); 244 } 245 ``` 246 247 instead of: 248 249 ```c 250 { 251 int a = foobar(); 252 uint64_t x = 7; 253 } 254 ``` 255 256- Use `goto` for cleaning up, and only use it for that. I.e. you may only jump 257 to the end of a function, and little else. Never jump backwards! 258 259- To minimize strict aliasing violations, we prefer unions over casting. 260 261- Instead of using `memzero()`/`memset()` to initialize structs allocated on 262 the stack, please try to use c99 structure initializers. It's short, prettier 263 and actually even faster at execution. Hence: 264 265 ```c 266 struct foobar t = { 267 .foo = 7, 268 .bar = "bazz", 269 }; 270 ``` 271 272 instead of: 273 274 ```c 275 struct foobar t; 276 zero(t); 277 t.foo = 7; 278 t.bar = "bazz"; 279 ``` 280 281- To implement an endless loop, use `for (;;)` rather than `while (1)`. The 282 latter is a bit ugly anyway, since you probably really meant `while 283 (true)`. To avoid the discussion what the right always-true expression for an 284 infinite while loop is, our recommendation is to simply write it without any 285 such expression by using `for (;;)`. 286 287- To determine the length of a constant string `"foo"`, don't bother with 288 `sizeof("foo")-1`, please use `strlen()` instead (both gcc and clang optimize 289 the call away for fixed strings). The only exception is when declaring an 290 array. In that case use `STRLEN()`, which evaluates to a static constant and 291 doesn't force the compiler to create a VLA. 292 293- Please use C's downgrade-to-bool feature only for expressions that are 294 actually booleans (or "boolean-like"), and not for variables that are really 295 numeric. Specifically, if you have an `int b` and it's only used in a boolean 296 sense, by all means check its state with `if (b) …` — but if `b` can actually 297 have more than two semantic values, and you want to compare for non-zero, 298 then please write that explicitly with `if (b != 0) …`. This helps readability 299 as the value range and semantical behaviour is directly clear from the 300 condition check. As a special addition: when dealing with pointers which you 301 want to check for non-NULL-ness, you may also use downgrade-to-bool feature. 302 303- Please do not use yoda comparisons, i.e. please prefer the more readable `if 304 (a == 7)` over the less readable `if (7 == a)`. 305 306## Destructors 307 308- The destructors always deregister the object from the next bigger object, not 309 the other way around. 310 311- For robustness reasons, destructors should be able to destruct 312 half-initialized objects, too. 313 314- When you define a destructor or `unref()` call for an object, please accept a 315 `NULL` object and simply treat this as NOP. This is similar to how libc 316 `free()` works, which accepts `NULL` pointers and becomes a NOP for them. By 317 following this scheme a lot of `if` checks can be removed before invoking 318 your destructor, which makes the code substantially more readable and robust. 319 320- Related to this: when you define a destructor or `unref()` call for an 321 object, please make it return the same type it takes and always return `NULL` 322 from it. This allows writing code like this: 323 324 ```c 325 p = foobar_unref(p); 326 ``` 327 328 which will always work regardless if `p` is initialized or not, and 329 guarantees that `p` is `NULL` afterwards, all in just one line. 330 331## Error Handling 332 333- Error codes are returned as negative `Exxx`. e.g. `return -EINVAL`. There are 334 some exceptions: for constructors, it is OK to return `NULL` on OOM. For 335 lookup functions, `NULL` is fine too for "not found". 336 337 Be strict with this. When you write a function that can fail due to more than 338 one cause, it *really* should have an `int` as the return value for the error 339 code. 340 341- libc system calls typically return -1 on error (with the error code in 342 `errno`), and >= 0 on success. Use the RET_NERRNO() helper if you are looking 343 for a simple way to convert this libc style error returning into systemd 344 style error returning. e.g. 345 346 ```c 347 … 348 r = RET_NERRNO(unlink(t)); 349 … 350 ``` 351 352 or 353 354 ```c 355 … 356 r = RET_NERRNO(open("/some/file", O_RDONLY|O_CLOEXEC)); 357 … 358 ``` 359 360- Do not bother with error checking whether writing to stdout/stderr worked. 361 362- Do not log errors from "library" code, only do so from "main program" 363 code. (With one exception: it is OK to log with DEBUG level from any code, 364 with the exception of maybe inner loops). 365 366- In public API calls, you **must** validate all your input arguments for 367 programming error with `assert_return()` and return a sensible return 368 code. In all other calls, it is recommended to check for programming errors 369 with a more brutal `assert()`. We are more forgiving to public users than for 370 ourselves! Note that `assert()` and `assert_return()` really only should be 371 used for detecting programming errors, not for runtime errors. `assert()` and 372 `assert_return()` by usage of `_likely_()` inform the compiler that it should 373 not expect these checks to fail, and they inform fellow programmers about the 374 expected validity and range of parameters. 375 376- When you invoke certain calls like `unlink()`, or `mkdir_p()` and you know it 377 is safe to ignore the error it might return (because a later call would 378 detect the failure anyway, or because the error is in an error path and you 379 thus couldn't do anything about it anyway), then make this clear by casting 380 the invocation explicitly to `(void)`. Code checks like Coverity understand 381 that, and will not complain about ignored error codes. Hence, please use 382 this: 383 384 ```c 385 (void) unlink("/foo/bar/baz"); 386 ``` 387 388 instead of just this: 389 390 ```c 391 unlink("/foo/bar/baz"); 392 ``` 393 394 When returning from a `void` function, you may also want to shorten the error 395 path boilerplate by returning a function invocation cast to `(void)` like so: 396 397 ```c 398 if (condition_not_met) 399 return (void) log_tests_skipped("Cannot run ..."); 400 ``` 401 402 Don't cast function calls to `(void)` that return no error 403 conditions. Specifically, the various `xyz_unref()` calls that return a 404 `NULL` object shouldn't be cast to `(void)`, since not using the return value 405 does not hide any errors. 406 407- When returning a return code from `main()`, please preferably use 408 `EXIT_FAILURE` and `EXIT_SUCCESS` as defined by libc. 409 410## Logging 411 412- For every function you add, think about whether it is a "logging" function or 413 a "non-logging" function. "Logging" functions do (non-debug) logging on their 414 own, "non-logging" functions never log on their own (except at debug level) 415 and expect their callers to log. All functions in "library" code, i.e. in 416 `src/shared/` and suchlike must be "non-logging". Every time a "logging" 417 function calls a "non-logging" function, it should log about the resulting 418 errors. If a "logging" function calls another "logging" function, then it 419 should not generate log messages, so that log messages are not generated 420 twice for the same errors. (Note that debug level logging — at syslog level 421 `LOG_DEBUG` — is not considered logging in this context, debug logging is 422 generally always fine and welcome.) 423 424- If possible, do a combined log & return operation: 425 426 ```c 427 r = operation(...); 428 if (r < 0) 429 return log_(error|warning|notice|...)_errno(r, "Failed to ...: %m"); 430 ``` 431 432 If the error value is "synthetic", i.e. it was not received from 433 the called function, use `SYNTHETIC_ERRNO` wrapper to tell the logging 434 system to not log the errno value, but still return it: 435 436 ```c 437 n = read(..., s, sizeof s); 438 if (n != sizeof s) 439 return log_error_errno(SYNTHETIC_ERRNO(EIO), "Failed to read ..."); 440 ``` 441 442## Memory Allocation 443 444- Always check OOM. There is no excuse. In program code, you can use 445 `log_oom()` for then printing a short message, but not in "library" code. 446 447- Avoid fixed-size string buffers, unless you really know the maximum size and 448 that maximum size is small. It is often nicer to use dynamic memory, 449 `alloca_safe()` or VLAs. If you do allocate fixed-size strings on the stack, 450 then it is probably only OK if you either use a maximum size such as 451 `LINE_MAX`, or count in detail the maximum size a string can 452 have. (`DECIMAL_STR_MAX` and `DECIMAL_STR_WIDTH` macros are your friends for 453 this!) 454 455 Or in other words, if you use `char buf[256]` then you are likely doing 456 something wrong! 457 458- Make use of `_cleanup_free_` and friends. It makes your code much nicer to 459 read (and shorter)! 460 461- Do not use `alloca()`, `strdupa()` or `strndupa()` directly. Use 462 `alloca_safe()`, `strdupa_safe()` or `strndupa_safe()` instead. (The 463 difference is that the latter include an assertion that the specified size is 464 below a safety threshold, so that the program rather aborts than runs into 465 possible stack overruns.) 466 467- Use `alloca_safe()`, but never forget that it is not OK to invoke 468 `alloca_safe()` within a loop or within function call 469 parameters. `alloca_safe()` memory is released at the end of a function, and 470 not at the end of a `{}` block. Thus, if you invoke it in a loop, you keep 471 increasing the stack pointer without ever releasing memory again. (VLAs have 472 better behavior in this case, so consider using them as an alternative.) 473 Regarding not using `alloca_safe()` within function parameters, see the BUGS 474 section of the `alloca(3)` man page. 475 476- If you want to concatenate two or more strings, consider using `strjoina()` 477 or `strjoin()` rather than `asprintf()`, as the latter is a lot slower. This 478 matters particularly in inner loops (but note that `strjoina()` cannot be 479 used there). 480 481## Runtime Behaviour 482 483- Avoid leaving long-running child processes around, i.e. `fork()`s that are 484 not followed quickly by an `execv()` in the child. Resource management is 485 unclear in this case, and memory CoW will result in unexpected penalties in 486 the parent much, much later on. 487 488- Don't block execution for arbitrary amounts of time using `usleep()` or a 489 similar call, unless you really know what you do. Just "giving something some 490 time", or so is a lazy excuse. Always wait for the proper event, instead of 491 doing time-based poll loops. 492 493- Whenever installing a signal handler, make sure to set `SA_RESTART` for it, 494 so that interrupted system calls are automatically restarted, and we minimize 495 hassles with handling `EINTR` (in particular as `EINTR` handling is pretty 496 broken on Linux). 497 498- When applying C-style unescaping as well as specifier expansion on the same 499 string, always apply the C-style unescaping first, followed by the specifier 500 expansion. When doing the reverse, make sure to escape `%` in specifier-style 501 first (i.e. `%` → `%%`), and then do C-style escaping where necessary. 502 503- Be exceptionally careful when formatting and parsing floating point 504 numbers. Their syntax is locale dependent (i.e. `5.000` in en_US is generally 505 understood as 5, while in de_DE as 5000.). 506 507- Make sure to enforce limits on every user controllable resource. If the user 508 can allocate resources in your code, your code must enforce some form of 509 limits after which it will refuse operation. It's fine if it is hard-coded 510 (at least initially), but it needs to be there. This is particularly 511 important for objects that unprivileged users may allocate, but also matters 512 for everything else any user may allocate. 513 514## Types 515 516- Think about the types you use. If a value cannot sensibly be negative, do not 517 use `int`, but use `unsigned`. 518 519- Use `char` only for actual characters. Use `uint8_t` or `int8_t` when you 520 actually mean a byte-sized signed or unsigned integers. When referring to a 521 generic byte, we generally prefer the unsigned variant `uint8_t`. Do not use 522 types based on `short`. They *never* make sense. Use `int`, `long`, `long 523 long`, all in unsigned and signed fashion, and the fixed-size types 524 `uint8_t`, `uint16_t`, `uint32_t`, `uint64_t`, `int8_t`, `int16_t`, `int32_t` 525 and so on, as well as `size_t`, but nothing else. Do not use kernel types 526 like `u32` and so on, leave that to the kernel. 527 528- Stay uniform. For example, always use `usec_t` for time values. Do not mix 529 `usec` and `msec`, and `usec` and whatnot. 530 531- Never use the `off_t` type, and particularly avoid it in public APIs. It's 532 really weirdly defined, as it usually is 64-bit and we don't support it any 533 other way, but it could in theory also be 32-bit. Which one it is depends on 534 a compiler switch chosen by the compiled program, which hence corrupts APIs 535 using it unless they can also follow the program's choice. Moreover, in 536 systemd we should parse values the same way on all architectures and cannot 537 expose `off_t` values over D-Bus. To avoid any confusion regarding conversion 538 and ABIs, always use simply `uint64_t` directly. 539 540- Unless you allocate an array, `double` is always a better choice than 541 `float`. Processors speak `double` natively anyway, so there is no speed 542 benefit, and on calls like `printf()` `float`s get promoted to `double`s 543 anyway, so there is no point. 544 545- Use the bool type for booleans, not integers. One exception: in public 546 headers (i.e those in `src/systemd/sd-*.h`) use integers after all, as `bool` 547 is C99 and in our public APIs we try to stick to C89 (with a few extensions; 548 also see above). 549 550## Deadlocks 551 552- Do not issue NSS requests (that includes user name and hostname lookups) 553 from PID 1 as this might trigger deadlocks when those lookups involve 554 synchronously talking to services that we would need to start up. 555 556- Do not synchronously talk to any other service from PID 1, due to risk of 557 deadlocks. 558 559## File Descriptors 560 561- When you allocate a file descriptor, it should be made `O_CLOEXEC` right from 562 the beginning, as none of our files should leak to forked binaries by 563 default. Hence, whenever you open a file, `O_CLOEXEC` must be specified, 564 right from the beginning. This also applies to sockets. Effectively, this 565 means that all invocations to: 566 567 - `open()` must get `O_CLOEXEC` passed, 568 - `socket()` and `socketpair()` must get `SOCK_CLOEXEC` passed, 569 - `recvmsg()` must get `MSG_CMSG_CLOEXEC` set, 570 - `F_DUPFD_CLOEXEC` should be used instead of `F_DUPFD`, and so on, 571 - invocations of `fopen()` should take `e`. 572 573- It's a good idea to use `O_NONBLOCK` when opening 'foreign' regular files, 574 i.e. file system objects that are supposed to be regular files whose paths 575 were specified by the user and hence might actually refer to other types of 576 file system objects. This is a good idea so that we don't end up blocking on 577 'strange' file nodes, for example if the user pointed us to a FIFO or device 578 node which may block when opening. Moreover even for actual regular files 579 `O_NONBLOCK` has a benefit: it bypasses any mandatory lock that might be in 580 effect on the regular file. If in doubt consider turning off `O_NONBLOCK` 581 again after opening. 582 583## Command Line 584 585- If you parse a command line, and want to store the parsed parameters in 586 global variables, please consider prefixing their names with `arg_`. We have 587 been following this naming rule in most of our tools, and we should continue 588 to do so, as it makes it easy to identify command line parameter variables, 589 and makes it clear why it is OK that they are global variables. 590 591- Command line option parsing: 592 - Do not print full `help()` on error, be specific about the error. 593 - Do not print messages to stdout on error. 594 - Do not POSIX_ME_HARDER unless necessary, i.e. avoid `+` in option string. 595 596## Exporting Symbols 597 598- Variables and functions **must** be static, unless they have a prototype, and 599 are supposed to be exported. 600 601- Public API calls (i.e. functions exported by our shared libraries) 602 must be marked `_public_` and need to be prefixed with `sd_`. No 603 other functions should be prefixed like that. 604 605- When exposing public C APIs, be careful what function parameters you make 606 `const`. For example, a parameter taking a context object should probably not 607 be `const`, even if you are writing an otherwise read-only accessor function 608 for it. The reason is that making it `const` fixates the contract that your 609 call won't alter the object ever, as part of the API. However, that's often 610 quite a promise, given that this even prohibits object-internal caching or 611 lazy initialization of object variables. Moreover, it's usually not too 612 useful for client applications. Hence, please be careful and avoid `const` on 613 object parameters, unless you are very sure `const` is appropriate. 614 615## Referencing Concepts 616 617- When referring to a configuration file option in the documentation and such, 618 please always suffix it with `=`, to indicate that it is a configuration file 619 setting. 620 621- When referring to a command line option in the documentation and such, please 622 always prefix with `--` or `-` (as appropriate), to indicate that it is a 623 command line option. 624 625- When referring to a file system path that is a directory, please always 626 suffix it with `/`, to indicate that it is a directory, not a regular file 627 (or other file system object). 628 629## Functions to Avoid 630 631- Use `memzero()` or even better `zero()` instead of `memset(..., 0, ...)` 632 633- Please use `streq()` and `strneq()` instead of `strcmp()`, `strncmp()` where 634 applicable (i.e. wherever you just care about equality/inequality, not about 635 the sorting order). 636 637- Never use `strtol()`, `atoi()` and similar calls. Use `safe_atoli()`, 638 `safe_atou32()` and suchlike instead. They are much nicer to use in most 639 cases and correctly check for parsing errors. 640 641- `htonl()`/`ntohl()` and `htons()`/`ntohs()` are weird. Please use `htobe32()` 642 and `htobe16()` instead, it's much more descriptive, and actually says what 643 really is happening, after all `htonl()` and `htons()` don't operate on 644 `long`s and `short`s as their name would suggest, but on `uint32_t` and 645 `uint16_t`. Also, "network byte order" is just a weird name for "big endian", 646 hence we might want to call it "big endian" right-away. 647 648- Please never use `dup()`. Use `fcntl(fd, F_DUPFD_CLOEXEC, 3)` instead. For 649 two reasons: first, you want `O_CLOEXEC` set on the new `fd` (see 650 above). Second, `dup()` will happily duplicate your `fd` as 0, 1, 2, 651 i.e. stdin, stdout, stderr, should those `fd`s be closed. Given the special 652 semantics of those `fd`s, it's probably a good idea to avoid 653 them. `F_DUPFD_CLOEXEC` with `3` as parameter avoids them. 654 655- Don't use `fgets()`, it's too hard to properly handle errors such as overly 656 long lines. Use `read_line()` instead, which is our own function that handles 657 this much more nicely. 658 659- Don't invoke `exit()`, ever. It is not replacement for proper error 660 handling. Please escalate errors up your call chain, and use normal `return` 661 to exit from the main function of a process. If you `fork()`ed off a child 662 process, please use `_exit()` instead of `exit()`, so that the exit handlers 663 are not run. 664 665- We never use the POSIX version of `basename()` (which glibc defines in 666 `libgen.h`), only the GNU version (which glibc defines in `string.h`). The 667 only reason to include `libgen.h` is because `dirname()` is needed. Every 668 time you need that please immediately undefine `basename()`, and add a 669 comment about it, so that no code ever ends up using the POSIX version! 670 671- Never use `FILENAME_MAX`. Use `PATH_MAX` instead (for checking maximum size 672 of paths) and `NAME_MAX` (for checking maximum size of filenames). 673 `FILENAME_MAX` is not POSIX, and is a confusingly named alias for `PATH_MAX` 674 on Linux. Note that `NAME_MAX` does not include space for a trailing `NUL`, 675 but `PATH_MAX` does. UNIX FTW! 676 677## Committing to git 678 679- Commit message subject lines should be prefixed with an appropriate component 680 name of some kind. For example "journal: ", "nspawn: " and so on. 681 682- Do not use "Signed-Off-By:" in your commit messages. That's a kernel thing we 683 don't do in the systemd project. 684