compatibility. Since we typically want to allow adding new enum values to an
existing enum type with later API versions, please use the
`_SD_ENUM_FORCE_S64()` macro in the enum definition, which forces the size of
- the enum to be signed 64bit wide.
+ the enum to be signed 64-bit wide.
- Empty lines to separate code blocks are a good thing, please add them
abundantly. However, please stick to one at a time, i.e. multiple empty lines
if (r < 0)
return log_error_errno(r, "Some function failed: %m");
/* → empty line here is good */
+ ```
- In shell scripts, do not use whitespace after the redirection operator
(`>some/file` instead of `> some/file`, `<<EOF` instead of `<< EOF`).
t.bar = "bazz";
```
-- To implement an endless loop, use `for (;;)` rather than `while (1)`. The
+- To implement an endless loop, use `for (;;)` rather than `while (1)`. The
latter is a bit ugly anyway, since you probably really meant `while
(true)`. To avoid the discussion what the right always-true expression for an
infinite while loop is, our recommendation is to simply write it without any
sure to imply `AT_EMPTY_PATH` if an empty or `NULL` path argument is
specified (and convert that latter to an empty string). This differs from the
underlying kernel semantics, where `AT_EMPTY_PATH` must always be specified
- explicitly, and `NULL` is not acepted as path.
+ explicitly, and `NULL` is not accepted as path.
## Command Line
- Reasonable use of non-ASCII Unicode UTF-8 characters in code comments is
welcome. If your code comment contains an emoji or two this will certainly
brighten the day of the occasional reviewer of your code. Really! 😊
+
+## Threading
+
+- We generally avoid using threads, to the level this is possible. In
+ particular in the service manager/PID 1 threads are not OK to use. This is
+ because you cannot mix memory allocation in threads with use of glibc's
+ `clone()` call, or manual `clone()`/`clone3()` system call wrappers. Only
+ glibc's own `fork()` call will properly synchronize the memory allocation
+ locks around the process clone operation. This means that if a process is
+ cloned via `clone()`/`clone3()` and another thread currently has the
+ `malloc()` lock taken, it will be cloned in locked state to the child, and
+ thus can never be acquired in the child, leading to deadlocks. Hence, when
+ using `clone()`/`clone3()` there are only two ways out: never use threads in the
+ parent, or never do memory allocation in the child. For our uses we need
+ `clone()`/`clone3()` and hence decided to avoid threads. Of course, sometimes the
+ concurrency threads allow is beneficial, however we suggest forking off
+ worker *processes* rather than worker *threads* for this purpose, ideally
+ even with an `execve()` to remove the CoW trap situation `fork()` easily
+ triggers.
+
+- A corollary of the above is: never use `clone()` where a `fork()` would do
+ too. Also consider using `posix_spawn()` which combines `clone()` +
+ `execve()` into one and has nice properties since it avoids becoming a CoW
+ trap by using `CLONE_VFORK` and `CLONE_VM` together.
+
+- While we avoid forking off threads on our own, writing thread-safe code is a
+ good idea where it might end up running inside of libsystemd.so or
+ similar. Hence, use TLS (i.e. `thread_local`) where appropriate, and maybe
+ the occasional `pthread_once()`.
+
+## Tests
+
+- Use the assertion macros from `tests.h` (`ASSERT_GE()`, `ASSERT_OK()`, ...) to
+ make sure a descriptive error is logged when an assertion fails. If no assertion
+ macro exists for your specific use case, please add a new assertion macro in a
+ separate commit.
+
+- When modifying existing tests, please convert the test to use the new assertion
+ macros from `tests.h` if it is not already using those.
projects / thirdparty / systemd.git / blobdiff