diff options
author | Lennart Poettering <lennart@poettering.net> | 2019-04-12 16:38:14 +0200 |
---|---|---|
committer | Lennart Poettering <lennart@poettering.net> | 2019-04-12 16:38:14 +0200 |
commit | b065e1f176267245dfb18ee8bc2bd675d382f519 (patch) | |
tree | f9cc6ef0c1d88916d7775c65e4fcee9452fe40de /docs/CODING_STYLE.md | |
parent | 831781b9c9a9c285c726f2e8e199dc5c144e9ed6 (diff) | |
download | systemd-b065e1f176267245dfb18ee8bc2bd675d382f519.tar.gz |
CODING_STYLE: Split out section about error handling
Diffstat (limited to 'docs/CODING_STYLE.md')
-rw-r--r-- | docs/CODING_STYLE.md | 104 |
1 files changed, 52 insertions, 52 deletions
diff --git a/docs/CODING_STYLE.md b/docs/CODING_STYLE.md index 88e40fc81e..20f9e10e7a 100644 --- a/docs/CODING_STYLE.md +++ b/docs/CODING_STYLE.md @@ -82,21 +82,6 @@ title: Coding Style - For robustness reasons, destructors should be able to destruct half-initialized objects, too. -- Error codes are returned as negative `Exxx`. e.g. `return -EINVAL`. There - are some exceptions: for constructors, it is OK to return `NULL` on - OOM. For lookup functions, `NULL` is fine too for "not found". - - Be strict with this. When you write a function that can fail due to - more than one cause, it *really* should have an `int` as the return value - for the error code. - -- Do not bother with error checking whether writing to stdout/stderr - worked. - -- Do not log errors from "library" code, only do so from "main - program" code. (With one exception: it is OK to log with DEBUG level - from any code, with the exception of maybe inner loops). - - Do not issue NSS requests (that includes user name and host name lookups) from PID 1 as this might trigger deadlocks when those lookups involve synchronously talking to services that we would need @@ -138,17 +123,6 @@ title: Coding Style must be marked `_public_` and need to be prefixed with `sd_`. No other functions should be prefixed like that. -- In public API calls, you **must** validate all your input arguments for - programming error with `assert_return()` and return a sensible return - code. In all other calls, it is recommended to check for programming - errors with a more brutal `assert()`. We are more forgiving to public - users than for ourselves! Note that `assert()` and `assert_return()` - really only should be used for detecting programming errors, not for - runtime errors. `assert()` and `assert_return()` by usage of `_likely_()` - inform the compiler that he should not expect these checks to fail, - and they inform fellow programmers about the expected validity and - range of parameters. - - For every function you add, think about whether it is a "logging" function or a "non-logging" function. "Logging" functions do logging on their own, "non-logging" function never log on their own and @@ -196,29 +170,6 @@ title: Coding Style failure. Use temporary variables for these cases and change the passed in variables only on success. -- When you invoke certain calls like `unlink()`, or `mkdir_p()` and you - know it is safe to ignore the error it might return (because a later - call would detect the failure anyway, or because the error is in an - error path and you thus couldn't do anything about it anyway), then - make this clear by casting the invocation explicitly to `(void)`. Code - checks like Coverity understand that, and will not complain about - ignored error codes. Hence, please use this: - - ```c - (void) unlink("/foo/bar/baz"); - ``` - - instead of just this: - - ```c - unlink("/foo/bar/baz"); - ``` - - Don't cast function calls to `(void)` that return no error - conditions. Specifically, the various `xyz_unref()` calls that return a `NULL` - object shouldn't be cast to `(void)`, since not using the return value does not - hide any errors. - - When you define a destructor or `unref()` call for an object, please accept a `NULL` object and simply treat this as NOP. This is similar to how libc `free()` works, which accepts `NULL` pointers and becomes a @@ -257,9 +208,6 @@ title: Coding Style t.bar = "bazz"; ``` -- When returning a return code from `main()`, please preferably use - `EXIT_FAILURE` and `EXIT_SUCCESS` as defined by libc. - - The order in which header files are included doesn't matter too much. systemd-internal headers must not rely on an include order, so it is safe to include them in any order possible. @@ -378,6 +326,58 @@ title: Coding Style expansion. When doing the reverse, make sure to escape `%` in specifier-style first (i.e. `%` → `%%`), and then do C-style escaping where necessary. +## Error Handling + +- Error codes are returned as negative `Exxx`. e.g. `return -EINVAL`. There are + some exceptions: for constructors, it is OK to return `NULL` on OOM. For + lookup functions, `NULL` is fine too for "not found". + + Be strict with this. When you write a function that can fail due to more than + one cause, it *really* should have an `int` as the return value for the error + code. + +- Do not bother with error checking whether writing to stdout/stderr worked. + +- Do not log errors from "library" code, only do so from "main program" + code. (With one exception: it is OK to log with DEBUG level from any code, + with the exception of maybe inner loops). + +- In public API calls, you **must** validate all your input arguments for + programming error with `assert_return()` and return a sensible return + code. In all other calls, it is recommended to check for programming errors + with a more brutal `assert()`. We are more forgiving to public users than for + ourselves! Note that `assert()` and `assert_return()` really only should be + used for detecting programming errors, not for runtime errors. `assert()` and + `assert_return()` by usage of `_likely_()` inform the compiler that he should + not expect these checks to fail, and they inform fellow programmers about the + expected validity and range of parameters. + +- When you invoke certain calls like `unlink()`, or `mkdir_p()` and you know it + is safe to ignore the error it might return (because a later call would + detect the failure anyway, or because the error is in an error path and you + thus couldn't do anything about it anyway), then make this clear by casting + the invocation explicitly to `(void)`. Code checks like Coverity understand + that, and will not complain about ignored error codes. Hence, please use + this: + + ```c + (void) unlink("/foo/bar/baz"); + ``` + + instead of just this: + + ```c + unlink("/foo/bar/baz"); + ``` + + Don't cast function calls to `(void)` that return no error + conditions. Specifically, the various `xyz_unref()` calls that return a + `NULL` object shouldn't be cast to `(void)`, since not using the return value + does not hide any errors. + +- When returning a return code from `main()`, please preferably use + `EXIT_FAILURE` and `EXIT_SUCCESS` as defined by libc. + ## Memory Allocation - Always check OOM. There is no excuse. In program code, you can use |