]>
Commit | Line | Data |
---|---|---|
798d3a52 ZJS |
1 | - 8ch indent, no tabs, except for files in man/ which are 2ch indent, |
2 | and still no tabs | |
60918275 | 3 | |
3fdbc820 LP |
4 | - Don't break code lines too eagerly. We do *not* force line breaks at |
5 | 80ch, all of today's screens should be much larger than that. But | |
6 | then again, don't overdo it, ~140ch should be enough really. | |
7 | ||
c170f3a4 | 8 | - Variables and functions *must* be static, unless they have a |
f168c273 | 9 | prototype, and are supposed to be exported. |
60918275 | 10 | |
d3a48513 LP |
11 | - structs in MixedCase (with exceptions, such as public API structs), |
12 | variables + functions in lower_case. | |
c170f3a4 LP |
13 | |
14 | - The destructors always unregister the object from the next bigger | |
60918275 LP |
15 | object, not the other way around |
16 | ||
8d0e0ddd | 17 | - To minimize strict aliasing violations, we prefer unions over casting |
60918275 | 18 | |
8d0e0ddd | 19 | - For robustness reasons, destructors should be able to destruct |
60918275 LP |
20 | half-initialized objects, too |
21 | ||
61f33134 | 22 | - Error codes are returned as negative Exxx. e.g. return -EINVAL. There |
8d0e0ddd JE |
23 | are some exceptions: for constructors, it is OK to return NULL on |
24 | OOM. For lookup functions, NULL is fine too for "not found". | |
c170f3a4 LP |
25 | |
26 | Be strict with this. When you write a function that can fail due to | |
27 | more than one cause, it *really* should have "int" as return value | |
28 | for the error code. | |
29 | ||
8e5edf8d | 30 | - Do not bother with error checking whether writing to stdout/stderr |
d3a48513 | 31 | worked. |
c170f3a4 LP |
32 | |
33 | - Do not log errors from "library" code, only do so from "main | |
8e5edf8d | 34 | program" code. (With one exception: it is OK to log with DEBUG level |
d3a48513 | 35 | from any code, with the exception of maybe inner loops). |
c170f3a4 | 36 | |
8e5edf8d | 37 | - Always check OOM. There is no excuse. In program code, you can use |
d3a48513 | 38 | "log_oom()" for then printing a short message, but not in "library" code. |
debf93a4 LP |
39 | |
40 | - Do not issue NSS requests (that includes user name and host name | |
d3a48513 LP |
41 | lookups) from PID 1 as this might trigger deadlocks when those |
42 | lookups involve synchronously talking to services that we would need | |
43 | to start up | |
debf93a4 | 44 | |
8e5edf8d | 45 | - Do not synchronously talk to any other service from PID 1, due to |
d3a48513 | 46 | risk of deadlocks |
c170f3a4 | 47 | |
45df8656 | 48 | - Avoid fixed-size string buffers, unless you really know the maximum |
c170f3a4 | 49 | size and that maximum size is small. They are a source of errors, |
45df8656 JE |
50 | since they possibly result in truncated strings. It is often nicer |
51 | to use dynamic memory, alloca() or VLAs. If you do allocate fixed-size | |
8e5edf8d | 52 | strings on the stack, then it is probably only OK if you either |
d3a48513 LP |
53 | use a maximum size such as LINE_MAX, or count in detail the maximum |
54 | size a string can have. (DECIMAL_STR_MAX and DECIMAL_STR_WIDTH | |
55 | macros are your friends for this!) | |
56 | ||
57 | Or in other words, if you use "char buf[256]" then you are likely | |
58 | doing something wrong! | |
c170f3a4 LP |
59 | |
60 | - Stay uniform. For example, always use "usec_t" for time | |
61f33134 | 61 | values. Do not mix usec and msec, and usec and whatnot. |
c170f3a4 LP |
62 | |
63 | - Make use of _cleanup_free_ and friends. It makes your code much | |
64 | nicer to read! | |
65 | ||
66 | - Be exceptionally careful when formatting and parsing floating point | |
67 | numbers. Their syntax is locale dependent (i.e. "5.000" in en_US is | |
68 | generally understood as 5, while on de_DE as 5000.). | |
69 | ||
70 | - Try to use this: | |
71 | ||
72 | void foo() { | |
73 | } | |
74 | ||
75 | instead of this: | |
76 | ||
77 | void foo() | |
78 | { | |
79 | } | |
80 | ||
8e5edf8d | 81 | But it is OK if you do not. |
c170f3a4 | 82 | |
61f33134 LP |
83 | - Single-line "if" blocks should not be enclosed in {}. Use this: |
84 | ||
85 | if (foobar) | |
86 | waldo(); | |
87 | ||
88 | instead of this: | |
89 | ||
90 | if (foobar) { | |
91 | waldo(); | |
92 | } | |
93 | ||
8e5edf8d | 94 | - Do not write "foo ()", write "foo()". |
c170f3a4 LP |
95 | |
96 | - Please use streq() and strneq() instead of strcmp(), strncmp() where applicable. | |
97 | ||
98 | - Please do not allocate variables on the stack in the middle of code, | |
99 | even if C99 allows it. Wrong: | |
100 | ||
101 | { | |
102 | a = 5; | |
103 | int b; | |
104 | b = a; | |
105 | } | |
106 | ||
107 | Right: | |
108 | ||
109 | { | |
110 | int b; | |
111 | a = 5; | |
112 | b = a; | |
113 | } | |
114 | ||
115 | - Unless you allocate an array, "double" is always the better choice | |
116 | than "float". Processors speak "double" natively anyway, so this is | |
45df8656 | 117 | no speed benefit, and on calls like printf() "float"s get promoted |
c170f3a4 LP |
118 | to "double"s anyway, so there is no point. |
119 | ||
8e5edf8d | 120 | - Do not invoke functions when you allocate variables on the stack. Wrong: |
c170f3a4 LP |
121 | |
122 | { | |
123 | int a = foobar(); | |
124 | uint64_t x = 7; | |
125 | } | |
126 | ||
127 | Right: | |
128 | ||
129 | { | |
130 | int a; | |
131 | uint64_t x = 7; | |
132 | ||
133 | a = foobar(); | |
134 | } | |
135 | ||
136 | - Use "goto" for cleaning up, and only use it for that. i.e. you may | |
d3a48513 LP |
137 | only jump to the end of a function, and little else. Never jump |
138 | backwards! | |
c170f3a4 LP |
139 | |
140 | - Think about the types you use. If a value cannot sensibly be | |
8e5edf8d | 141 | negative, do not use "int", but use "unsigned". |
c170f3a4 | 142 | |
8e5edf8d | 143 | - Do not use types like "short". They *never* make sense. Use ints, |
c170f3a4 | 144 | longs, long longs, all in unsigned+signed fashion, and the fixed |
8d0e0ddd | 145 | size types uint32_t and so on, as well as size_t, but nothing else. |
d3a48513 LP |
146 | |
147 | - Public API calls (i.e. functions exported by our shared libraries) | |
148 | must be marked "_public_" and need to be prefixed with "sd_". No | |
149 | other functions should be prefixed like that. | |
150 | ||
8d0e0ddd | 151 | - In public API calls, you *must* validate all your input arguments for |
d3a48513 | 152 | programming error with assert_return() and return a sensible return |
8d0e0ddd | 153 | code. In all other calls, it is recommended to check for programming |
d3a48513 LP |
154 | errors with a more brutal assert(). We are more forgiving to public |
155 | users then for ourselves! Note that assert() and assert_return() | |
156 | really only should be used for detecting programming errors, not for | |
157 | runtime errors. assert() and assert_return() by usage of _likely_() | |
8e5edf8d | 158 | inform the compiler that he should not expect these checks to fail, |
d3a48513 LP |
159 | and they inform fellow programmers about the expected validity and |
160 | range of parameters. | |
161 | ||
162 | - Never use strtol(), atoi() and similar calls. Use safe_atoli(), | |
163 | safe_atou32() and suchlike instead. They are much nicer to use in | |
164 | most cases and correctly check for parsing errors. | |
165 | ||
166 | - For every function you add, think about whether it is a "logging" | |
167 | function or a "non-logging" function. "Logging" functions do logging | |
168 | on their own, "non-logging" function never log on their own and | |
169 | expect their callers to log. All functions in "library" code, | |
06b643e7 | 170 | i.e. in src/shared/ and suchlike must be "non-logging". Every time a |
8d0e0ddd | 171 | "logging" function calls a "non-logging" function, it should log |
d3a48513 LP |
172 | about the resulting errors. If a "logging" function calls another |
173 | "logging" function, then it should not generate log messages, so | |
174 | that log messages are not generated twice for the same errors. | |
175 | ||
176 | - Avoid static variables, except for caches and very few other | |
177 | cases. Think about thread-safety! While most of our code is never | |
8d0e0ddd | 178 | used in threaded environments, at least the library code should make |
d3a48513 | 179 | sure it works correctly in them. Instead of doing a lot of locking |
8d0e0ddd | 180 | for that, we tend to prefer using TLS to do per-thread caching (which |
d3a48513 LP |
181 | only works for small, fixed-size cache objects), or we disable |
182 | caching for any thread that is not the main thread. Use | |
183 | is_main_thread() to detect whether the calling thread is the main | |
184 | thread. | |
601185b4 | 185 | |
7f8bf08f | 186 | - Command line option parsing: |
601185b4 ZJS |
187 | - Do not print full help() on error, be specific about the error. |
188 | - Do not print messages to stdout on error. | |
189 | - Do not POSIX_ME_HARDER unless necessary, i.e. avoid "+" in option string. | |
7f8bf08f LP |
190 | |
191 | - Do not write functions that clobber call-by-reference variables on | |
192 | failure. Use temporary variables for these cases and change the | |
193 | passed in variables only on success. | |
dd4540da LP |
194 | |
195 | - When you allocate a file descriptor, it should be made O_CLOEXEC | |
196 | right from the beginning, as none of our files should leak to forked | |
197 | binaries by default. Hence, whenever you open a file, O_CLOEXEC must | |
699eee62 LP |
198 | be specified, right from the beginning. This also applies to |
199 | sockets. Effectively this means that all invocations to: | |
200 | ||
201 | a) open() must get O_CLOEXEC passed | |
202 | b) socket() and socketpair() must get SOCK_CLOEXEC passed | |
203 | c) recvmsg() must get MSG_CMSG_CLOEXEC set | |
204 | d) F_DUPFD_CLOEXEC should be used instead of F_DUPFD, and so on | |
eef46c37 LP |
205 | |
206 | - We never use the XDG version of basename(). glibc defines it in | |
207 | libgen.h. The only reason to include that file is because dirname() | |
208 | is needed. Everytime you need that please immediately undefine | |
209 | basename(), and add a comment about it, so that no code ever ends up | |
210 | using the XDG version! |