- Don't break code lines too eagerly. We do *not* force line breaks at
80ch, all of today's screens should be much larger than that. But
- then again, don't overdo it, ~140ch should be enough really.
+ then again, don't overdo it, ~119ch should be enough really.
- Variables and functions *must* be static, unless they have a
prototype, and are supposed to be exported.
- Think about the types you use. If a value cannot sensibly be
negative, do not use "int", but use "unsigned".
-- Do not use types like "short". They *never* make sense. Use ints,
- longs, long longs, all in unsigned+signed fashion, and the fixed
- size types uint32_t and so on, as well as size_t, but nothing
- else. Do not use kernel types like u32 and so on, leave that to the
- kernel.
+- Use "char" only for actual characters. Use "uint8_t" or "int8_t"
+ when you actually mean a byte-sized signed or unsigned
+ integers. When referring to a generic byte, we generally prefer the
+ unsigned variant "uint8_t". Do not use types based on "short". They
+ *never* make sense. Use ints, longs, long longs, all in
+ unsigned+signed fashion, and the fixed size types
+ uint8_t/uint16_t/uint32_t/uint64_t/int8_t/int16_t/int32_t and so on,
+ as well as size_t, but nothing else. Do not use kernel types like
+ u32 and so on, leave that to the kernel.
- Public API calls (i.e. functions exported by our shared libraries)
must be marked "_public_" and need to be prefixed with "sd_". No
EXIT_FAILURE and EXIT_SUCCESS as defined by libc.
- The order in which header files are included doesn't matter too
- much. However, please try to include the headers of external
- libraries first (these are all headers enclosed in <>), followed by
- the headers of our own public headers (these are all headers
- starting with "sd-"), internal utility libraries from src/shared/,
- followed by the headers of the specific component. Or in other
- words:
-
- #include <stdio.h>
- #include "sd-daemon.h"
- #include "util.h"
- #include "frobnicator.h"
-
- Where stdio.h is a public glibc API, sd-daemon.h is a public API of
- our own, util.h is a utility library header from src/shared, and
- frobnicator.h is an placeholder name for any systemd component. The
- benefit of following this ordering is that more local definitions
- are always defined after more global ones. Thus, our local
- definitions will never "leak" into the global header files, possibly
- altering their effect due to #ifdeffery.
+ much. systemd-internal headers must not rely on an include order, so
+ it is safe to include them in any order possible.
+ However, to not clutter global includes, and to make sure internal
+ definitions will not affect global headers, please always include the
+ headers of external components first (these are all headers enclosed
+ in <>), followed by our own exported headers (usually everything
+ that's prefixed by "sd-"), and then followed by internal headers.
+ Furthermore, in all three groups, order all includes alphabetically
+ so duplicate includes can easily be detected.
+
+- 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 such expression by
+ using "for (;;)".
+
+- Never use the "off_t" type, and particularly avoid it in public
+ APIs. It's really weirdly defined, as it usually is 64bit and we
+ don't support it any other way, but it could in theory also be
+ 32bit. Which one it is depends on a compiler switch chosen by the
+ compiled program, which hence corrupts APIs using it unless they can
+ also follow the program's choice. Moreover, in systemd we should
+ parse values the same way on all architectures and cannot expose
+ off_t values over D-Bus. To avoid any confusion regarding conversion
+ and ABIs, always use simply uint64_t directly.
+
+- Commit message subject lines should be prefixed with an appropriate
+ component name of some kind. For example "journal: ", "nspawn: " and
+ so on.
+
+- Do not use "Signed-Off-By:" in your commit messages. That's a kernel
+ thing we don't do in the systemd project.
+
+- Avoid leaving long-running child processes around, i.e. fork()s that
+ are not followed quickly by an execv() in the child. Resource
+ management is unclear in this case, and memory CoW will result in
+ unexpected penalties in the parent much much later on.
+
+- Don't block execution for arbitrary amounts of time using usleep()
+ or a similar call, unless you really know what you do. Just "giving
+ something some time", or so is a lazy excuse. Always wait for the
+ proper event, instead of doing time-based poll loops.
+
+- To determine the length of a constant string "foo", don't bother
+ with sizeof("foo")-1, please use strlen("foo") directly. gcc knows
+ strlen() anyway and turns it into a constant expression if possible.
+
+- If you want to concatenate two or more strings, consider using
+ strjoin() rather than asprintf(), as the latter is a lot
+ slower. This matters particularly in inner loops.
+
+- Please avoid using global variables as much as you can. And if you
+ do use them make sure they are static at least, instead of
+ exported. Especially in library-like code it is important to avoid
+ global variables. Why are global variables bad? They usually hinder
+ generic reusability of code (since they break in threaded programs,
+ and usually would require locking there), and as the code using them
+ has side-effects make programs intransparent. That said, there are
+ many cases where they explicitly make a lot of sense, and are OK to
+ use. For example, the log level and target in log.c is stored in a
+ global variable, and that's OK and probably expected by most. Also
+ in many cases we cache data in global variables. If you add more
+ caches like this, please be careful however, and think about
+ threading. Only use static variables if you are sure that
+ thread-safety doesn't matter in your case. Alternatively consider
+ using TLS, which is pretty easy to use with gcc's "thread_local"
+ concept. It's also OK to store data that is inherently global in
+ global variables, for example data parsed from command lines, see
+ below.
+
+- If you parse a command line, and want to store the parsed parameters
+ in global variables, please consider prefixing their names with
+ "arg_". We have been following this naming rule in most of our
+ tools, and we should continue to do so, as it makes it easy to
+ identify command line parameter variables, and makes it clear why it
+ is OK that they are global variables.