doc/CODING_STYLE

   1 - 8ch indent, no tabs, except for files in man/ which are 2ch indent,
   2   and still no tabs
   3
   4 - We prefer /* comments */ over // comments in code you commit, please. This
   5   way // comments are left for developers to use for local, temporary
   6   commenting of code for debug purposes (i.e. uncommittable stuff), making such
   7   comments easily discernable from explanatory, documenting code comments
   8   (i.e. committable stuff).
   9
  10 - Don't break code lines too eagerly. We do *not* force line breaks at 80ch,
  11   all of today's screens should be much larger than that. But then again, don't
  12   overdo it, ~119ch should be enough really. The .editorconfig, .vimrc and
  13   .dir-locals.el files contained in the repository will set this limit up for
  14   you automatically, if you let them (as well as a few other things).
  15
  16 - Variables and functions *must* be static, unless they have a
  17   prototype, and are supposed to be exported.
  18
  19 - structs in MixedCase (with exceptions, such as public API structs),
  20   variables + functions in lower_case.
  21
  22 - The destructors always unregister the object from the next bigger
  23   object, not the other way around
  24
  25 - To minimize strict aliasing violations, we prefer unions over casting
  26
  27 - For robustness reasons, destructors should be able to destruct
  28   half-initialized objects, too
  29
  30 - Error codes are returned as negative Exxx. e.g. return -EINVAL. There
  31   are some exceptions: for constructors, it is OK to return NULL on
  32   OOM. For lookup functions, NULL is fine too for "not found".
  33
  34   Be strict with this. When you write a function that can fail due to
  35   more than one cause, it *really* should have "int" as return value
  36   for the error code.
  37
  38 - Do not bother with error checking whether writing to stdout/stderr
  39   worked.
  40
  41 - Do not log errors from "library" code, only do so from "main
  42   program" code. (With one exception: it is OK to log with DEBUG level
  43   from any code, with the exception of maybe inner loops).
  44
  45 - Always check OOM. There is no excuse. In program code, you can use
  46   "log_oom()" for then printing a short message, but not in "library" code.
  47
  48 - Do not issue NSS requests (that includes user name and host name
  49   lookups) from PID 1 as this might trigger deadlocks when those
  50   lookups involve synchronously talking to services that we would need
  51   to start up
  52
  53 - Do not synchronously talk to any other service from PID 1, due to
  54   risk of deadlocks
  55
  56 - Avoid fixed-size string buffers, unless you really know the maximum
  57   size and that maximum size is small. They are a source of errors,
  58   since they possibly result in truncated strings. It is often nicer
  59   to use dynamic memory, alloca() or VLAs. If you do allocate fixed-size
  60   strings on the stack, then it is probably only OK if you either
  61   use a maximum size such as LINE_MAX, or count in detail the maximum
  62   size a string can have. (DECIMAL_STR_MAX and DECIMAL_STR_WIDTH
  63   macros are your friends for this!)
  64
  65   Or in other words, if you use "char buf[256]" then you are likely
  66   doing something wrong!
  67
  68 - Stay uniform. For example, always use "usec_t" for time
  69   values. Do not mix usec and msec, and usec and whatnot.
  70
  71 - Make use of _cleanup_free_ and friends. It makes your code much
  72   nicer to read (and shorter)!
  73
  74 - Be exceptionally careful when formatting and parsing floating point
  75   numbers. Their syntax is locale dependent (i.e. "5.000" in en_US is
  76   generally understood as 5, while on de_DE as 5000.).
  77
  78 - Try to use this:
  79
  80       void foo() {
  81       }
  82
  83   instead of this:
  84
  85       void foo()
  86       {
  87       }
  88
  89   But it is OK if you do not.
  90
  91 - Single-line "if" blocks should not be enclosed in {}. Use this:
  92
  93   if (foobar)
  94           waldo();
  95
  96   instead of this:
  97
  98   if (foobar) {
  99           waldo();
 100   }
 101
 102 - Do not write "foo ()", write "foo()".
 103
 104 - Please use streq() and strneq() instead of strcmp(), strncmp() where
 105   applicable (i.e. wherever you just care about equality/inequality, not about
 106   the sorting order).
 107
 108 - Preferably allocate stack variables on the top of the block:
 109
 110   {
 111           int a, b;
 112
 113           a = 5;
 114           b = a;
 115   }
 116
 117 - Unless you allocate an array, "double" is always the better choice
 118   than "float". Processors speak "double" natively anyway, so this is
 119   no speed benefit, and on calls like printf() "float"s get promoted
 120   to "double"s anyway, so there is no point.
 121
 122 - Do not mix function invocations with variable definitions in one
 123   line. Wrong:
 124
 125   {
 126           int a = foobar();
 127           uint64_t x = 7;
 128   }
 129
 130   Right:
 131
 132   {
 133           int a;
 134           uint64_t x = 7;
 135
 136           a = foobar();
 137   }
 138
 139 - Use "goto" for cleaning up, and only use it for that. i.e. you may
 140   only jump to the end of a function, and little else. Never jump
 141   backwards!
 142
 143 - Think about the types you use. If a value cannot sensibly be
 144   negative, do not use "int", but use "unsigned".
 145
 146 - Use "char" only for actual characters. Use "uint8_t" or "int8_t"
 147   when you actually mean a byte-sized signed or unsigned
 148   integers. When referring to a generic byte, we generally prefer the
 149   unsigned variant "uint8_t". Do not use types based on "short". They
 150   *never* make sense. Use ints, longs, long longs, all in
 151   unsigned+signed fashion, and the fixed size types
 152   uint8_t/uint16_t/uint32_t/uint64_t/int8_t/int16_t/int32_t and so on,
 153   as well as size_t, but nothing else. Do not use kernel types like
 154   u32 and so on, leave that to the kernel.
 155
 156 - Public API calls (i.e. functions exported by our shared libraries)
 157   must be marked "_public_" and need to be prefixed with "sd_". No
 158   other functions should be prefixed like that.
 159
 160 - In public API calls, you *must* validate all your input arguments for
 161   programming error with assert_return() and return a sensible return
 162   code. In all other calls, it is recommended to check for programming
 163   errors with a more brutal assert(). We are more forgiving to public
 164   users than for ourselves! Note that assert() and assert_return()
 165   really only should be used for detecting programming errors, not for
 166   runtime errors. assert() and assert_return() by usage of _likely_()
 167   inform the compiler that he should not expect these checks to fail,
 168   and they inform fellow programmers about the expected validity and
 169   range of parameters.
 170
 171 - Never use strtol(), atoi() and similar calls. Use safe_atoli(),
 172   safe_atou32() and suchlike instead. They are much nicer to use in
 173   most cases and correctly check for parsing errors.
 174
 175 - For every function you add, think about whether it is a "logging"
 176   function or a "non-logging" function. "Logging" functions do logging
 177   on their own, "non-logging" function never log on their own and
 178   expect their callers to log. All functions in "library" code,
 179   i.e. in src/shared/ and suchlike must be "non-logging". Every time a
 180   "logging" function calls a "non-logging" function, it should log
 181   about the resulting errors. If a "logging" function calls another
 182   "logging" function, then it should not generate log messages, so
 183   that log messages are not generated twice for the same errors.
 184
 185 - Avoid static variables, except for caches and very few other
 186   cases. Think about thread-safety! While most of our code is never
 187   used in threaded environments, at least the library code should make
 188   sure it works correctly in them. Instead of doing a lot of locking
 189   for that, we tend to prefer using TLS to do per-thread caching (which
 190   only works for small, fixed-size cache objects), or we disable
 191   caching for any thread that is not the main thread. Use
 192   is_main_thread() to detect whether the calling thread is the main
 193   thread.
 194
 195 - Command line option parsing:
 196   - Do not print full help() on error, be specific about the error.
 197   - Do not print messages to stdout on error.
 198   - Do not POSIX_ME_HARDER unless necessary, i.e. avoid "+" in option string.
 199
 200 - Do not write functions that clobber call-by-reference variables on
 201   failure. Use temporary variables for these cases and change the
 202   passed in variables only on success.
 203
 204 - When you allocate a file descriptor, it should be made O_CLOEXEC
 205   right from the beginning, as none of our files should leak to forked
 206   binaries by default. Hence, whenever you open a file, O_CLOEXEC must
 207   be specified, right from the beginning. This also applies to
 208   sockets. Effectively this means that all invocations to:
 209
 210   a) open() must get O_CLOEXEC passed
 211   b) socket() and socketpair() must get SOCK_CLOEXEC passed
 212   c) recvmsg() must get MSG_CMSG_CLOEXEC set
 213   d) F_DUPFD_CLOEXEC should be used instead of F_DUPFD, and so on
 214   f) invocations of fopen() should take "e"
 215
 216 - We never use the POSIX version of basename() (which glibc defines it in
 217   libgen.h), only the GNU version (which glibc defines in string.h).
 218   The only reason to include libgen.h is because dirname()
 219   is needed. Every time you need that please immediately undefine
 220   basename(), and add a comment about it, so that no code ever ends up
 221   using the POSIX version!
 222
 223 - Use the bool type for booleans, not integers. One exception: in public
 224   headers (i.e those in src/systemd/sd-*.h) use integers after all, as "bool"
 225   is C99 and in our public APIs we try to stick to C89 (with a few extension).
 226
 227 - When you invoke certain calls like unlink(), or mkdir_p() and you
 228   know it is safe to ignore the error it might return (because a later
 229   call would detect the failure anyway, or because the error is in an
 230   error path and you thus couldn't do anything about it anyway), then
 231   make this clear by casting the invocation explicitly to (void). Code
 232   checks like Coverity understand that, and will not complain about
 233   ignored error codes. Hence, please use this:
 234
 235       (void) unlink("/foo/bar/baz");
 236
 237   instead of just this:
 238
 239       unlink("/foo/bar/baz");
 240
 241   Don't cast function calls to (void) that return no error
 242   conditions. Specifically, the various xyz_unref() calls that return a NULL
 243   object shouldn't be cast to (void), since not using the return value does not
 244   hide any errors.
 245
 246 - Don't invoke exit(), ever. It is not replacement for proper error
 247   handling. Please escalate errors up your call chain, and use normal
 248   "return" to exit from the main function of a process. If you
 249   fork()ed off a child process, please use _exit() instead of exit(),
 250   so that the exit handlers are not run.
 251
 252 - Please never use dup(). Use fcntl(fd, F_DUPFD_CLOEXEC, 3)
 253   instead. For two reason: first, you want O_CLOEXEC set on the new fd
 254   (see above). Second, dup() will happily duplicate your fd as 0, 1,
 255   2, i.e. stdin, stdout, stderr, should those fds be closed. Given the
 256   special semantics of those fds, it's probably a good idea to avoid
 257   them. F_DUPFD_CLOEXEC with "3" as parameter avoids them.
 258
 259 - When you define a destructor or unref() call for an object, please
 260   accept a NULL object and simply treat this as NOP. This is similar
 261   to how libc free() works, which accepts NULL pointers and becomes a
 262   NOP for them. By following this scheme a lot of if checks can be
 263   removed before invoking your destructor, which makes the code
 264   substantially more readable and robust.
 265
 266 - Related to this: when you define a destructor or unref() call for an
 267   object, please make it return the same type it takes and always
 268   return NULL from it. This allows writing code like this:
 269
 270             p = foobar_unref(p);
 271
 272   which will always work regardless if p is initialized or not, and
 273   guarantees that p is NULL afterwards, all in just one line.
 274
 275 - Use alloca(), but never forget that it is not OK to invoke alloca()
 276   within a loop or within function call parameters. alloca() memory is
 277   released at the end of a function, and not at the end of a {}
 278   block. Thus, if you invoke it in a loop, you keep increasing the
 279   stack pointer without ever releasing memory again. (VLAs have better
 280   behaviour in this case, so consider using them as an alternative.)
 281   Regarding not using alloca() within function parameters, see the
 282   BUGS section of the alloca(3) man page.
 283
 284 - Use memzero() or even better zero() instead of memset(..., 0, ...)
 285
 286 - Instead of using memzero()/memset() to initialize structs allocated
 287   on the stack, please try to use c99 structure initializers. It's
 288   short, prettier and actually even faster at execution. Hence:
 289
 290           struct foobar t = {
 291                   .foo = 7,
 292                   .bar = "bazz",
 293           };
 294
 295   instead of:
 296
 297           struct foobar t;
 298           zero(t);
 299           t.foo = 7;
 300           t.bar = "bazz";
 301
 302 - When returning a return code from main(), please preferably use
 303   EXIT_FAILURE and EXIT_SUCCESS as defined by libc.
 304
 305 - The order in which header files are included doesn't matter too
 306   much. systemd-internal headers must not rely on an include order, so
 307   it is safe to include them in any order possible.
 308   However, to not clutter global includes, and to make sure internal
 309   definitions will not affect global headers, please always include the
 310   headers of external components first (these are all headers enclosed
 311   in <>), followed by our own exported headers (usually everything
 312   that's prefixed by "sd-"), and then followed by internal headers.
 313   Furthermore, in all three groups, order all includes alphabetically
 314   so duplicate includes can easily be detected.
 315
 316 - To implement an endless loop, use "for (;;)" rather than "while
 317   (1)". The latter is a bit ugly anyway, since you probably really
 318   meant "while (true)"... To avoid the discussion what the right
 319   always-true expression for an infinite while() loop is our
 320   recommendation is to simply write it without any such expression by
 321   using "for (;;)".
 322
 323 - Never use the "off_t" type, and particularly avoid it in public
 324   APIs. It's really weirdly defined, as it usually is 64bit and we
 325   don't support it any other way, but it could in theory also be
 326   32bit. Which one it is depends on a compiler switch chosen by the
 327   compiled program, which hence corrupts APIs using it unless they can
 328   also follow the program's choice. Moreover, in systemd we should
 329   parse values the same way on all architectures and cannot expose
 330   off_t values over D-Bus. To avoid any confusion regarding conversion
 331   and ABIs, always use simply uint64_t directly.
 332
 333 - Commit message subject lines should be prefixed with an appropriate
 334   component name of some kind. For example "journal: ", "nspawn: " and
 335   so on.
 336
 337 - Do not use "Signed-Off-By:" in your commit messages. That's a kernel
 338   thing we don't do in the systemd project.
 339
 340 - Avoid leaving long-running child processes around, i.e. fork()s that
 341   are not followed quickly by an execv() in the child. Resource
 342   management is unclear in this case, and memory CoW will result in
 343   unexpected penalties in the parent much, much later on.
 344
 345 - Don't block execution for arbitrary amounts of time using usleep()
 346   or a similar call, unless you really know what you do. Just "giving
 347   something some time", or so is a lazy excuse. Always wait for the
 348   proper event, instead of doing time-based poll loops.
 349
 350 - To determine the length of a constant string "foo", don't bother
 351   with sizeof("foo")-1, please use STRLEN() instead.
 352
 353 - If you want to concatenate two or more strings, consider using
 354   strjoin() rather than asprintf(), as the latter is a lot
 355   slower. This matters particularly in inner loops.
 356
 357 - Please avoid using global variables as much as you can. And if you
 358   do use them make sure they are static at least, instead of
 359   exported. Especially in library-like code it is important to avoid
 360   global variables. Why are global variables bad? They usually hinder
 361   generic reusability of code (since they break in threaded programs,
 362   and usually would require locking there), and as the code using them
 363   has side-effects make programs non-transparent. That said, there are
 364   many cases where they explicitly make a lot of sense, and are OK to
 365   use. For example, the log level and target in log.c is stored in a
 366   global variable, and that's OK and probably expected by most. Also
 367   in many cases we cache data in global variables. If you add more
 368   caches like this, please be careful however, and think about
 369   threading. Only use static variables if you are sure that
 370   thread-safety doesn't matter in your case. Alternatively consider
 371   using TLS, which is pretty easy to use with gcc's "thread_local"
 372   concept. It's also OK to store data that is inherently global in
 373   global variables, for example data parsed from command lines, see
 374   below.
 375
 376 - If you parse a command line, and want to store the parsed parameters
 377   in global variables, please consider prefixing their names with
 378   "arg_". We have been following this naming rule in most of our
 379   tools, and we should continue to do so, as it makes it easy to
 380   identify command line parameter variables, and makes it clear why it
 381   is OK that they are global variables.
 382
 383 - When exposing public C APIs, be careful what function parameters you make
 384   "const". For example, a parameter taking a context object should probably not
 385   be "const", even if you are writing an otherwise read-only accessor function
 386   for it. The reason is that making it "const" fixates the contract that your
 387   call won't alter the object ever, as part of the API. However, that's often
 388   quite a promise, given that this even prohibits object-internal caching or
 389   lazy initialization of object variables. Moreover it's usually not too useful
 390   for client applications. Hence: please be careful and avoid "const" on object
 391   parameters, unless you are very sure "const" is appropriate.
 392
 393 - Make sure to enforce limits on every user controllable resource. If the user
 394   can allocate resources in your code, your code must enforce some form of
 395   limits after which it will refuse operation. It's fine if it is hard-coded (at
 396   least initially), but it needs to be there. This is particularly important
 397   for objects that unprivileged users may allocate, but also matters for
 398   everything else any user may allocated.
 399
 400 - htonl()/ntohl() and htons()/ntohs() are weird. Please use htobe32() and
 401   htobe16() instead, it's much more descriptive, and actually says what really
 402   is happening, after all htonl() and htons() don't operate on longs and
 403   shorts as their name would suggest, but on uint32_t and uint16_t. Also,
 404   "network byte order" is just a weird name for "big endian", hence we might
 405   want to call it "big endian" right-away.
 406
 407 - You might wonder what kind of common code belongs in src/shared/ and what
 408   belongs in src/basic/. The split is like this: anything that uses public APIs
 409   we expose (i.e. any of the sd-bus, sd-login, sd-id128, ... APIs) must be
 410   located in src/shared/. All stuff that only uses external libraries from
 411   other projects (such as glibc's APIs), or APIs from src/basic/ itself should
 412   be placed in src/basic/. Conversely, src/libsystemd/ may only use symbols
 413   from src/basic, but not from src/shared/. To summarize:
 414
 415   src/basic/      → may be used by all code in the tree
 416                   → may not use any code outside of src/basic/
 417
 418   src/libsystemd/ → may be used by all code in the tree, except for code in src/basic/
 419                   → may not use any code outside of src/basic/, src/libsystemd/
 420
 421   src/shared/     → may be used by all code in the tree, except for code in src/basic/, src/libsystemd/
 422                   → may not use any code outside of src/basic/, src/libsystemd/, src/shared/
 423
 424 - Our focus is on the GNU libc (glibc), not any other libcs. If other libcs are
 425   incompatible with glibc it's on them. However, if there are equivalent POSIX
 426   and Linux/GNU-specific APIs, we generally prefer the POSIX APIs. If there
 427   aren't, we are happy to use GNU or Linux APIs, and expect non-GNU
 428   implementations of libc to catch up with glibc.
 429
 430 - Whenever installing a signal handler, make sure to set SA_RESTART for it, so
 431   that interrupted system calls are automatically restarted, and we minimize
 432   hassles with handling EINTR (in particular as EINTR handling is pretty broken
 433   on Linux).
 434
 435 - When applying C-style unescaping as well as specifier expansion on the same
 436   string, always apply the C-style unescaping fist, followed by the specifier
 437   expansion. When doing the reverse, make sure to escape '%' in specifier-style
 438   first (i.e. '%' → '%%'), and then do C-style escaping where necessary.
 439
 440 - It's a good idea to use O_NONBLOCK when opening 'foreign' regular files, i.e
 441   file system objects that are supposed to be regular files whose paths where
 442   specified by the user and hence might actually refer to other types of file
 443   system objects. This is a good idea so that we don't end up blocking on
 444   'strange' file nodes, for example if the user pointed us to a FIFO or device
 445   node which may block when opening. Moreover even for actual regular files
 446   O_NONBLOCK has a benefit: it bypasses any mandatory lock that might be in
 447   effect on the regular file. If in doubt consider turning off O_NONBLOCK again
 448   after opening.
 449
 450 - When referring to a configuration file option in the documentation and such,
 451   please always suffix it with "=", to indicate that it is a configuration file
 452   setting.
 453
 454 - When referring to a command line option in the documentation and such, please
 455   always prefix with "--" or "-" (as appropriate), to indicate that it is a
 456   command line option.
 457
 458 - When referring to a file system path that is a directory, please always
 459   suffix it with "/", to indicate that it is a directory, not a regular file
 460   (or other file system object).