- 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
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 then for ourselves! Note that assert() and assert_return()
+ 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,
b) socket() and socketpair() must get SOCK_CLOEXEC passed
c) recvmsg() must get MSG_CMSG_CLOEXEC set
d) F_DUPFD_CLOEXEC should be used instead of F_DUPFD, and so on
+ f) invocations of fopen() should take "e"
- We never use the POSIX version of basename() (which glibc defines it in
libgen.h), only the GNU version (which glibc defines in string.h).
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.
+
- Don't invoke exit(), ever. It is not replacement for proper error
handling. Please escalate errors up your call chain, and use normal
"return" to exit from the main function of a process. If you
- 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.
+
+- When exposing public C APIs, be careful what function parameters you make
+ "const". For example, a parameter taking a context object should probably not
+ be "const", even if you are writing an other-wise read-only accessor function
+ for it. The reason is that making it "const" fixates the contract that your
+ call won't alter the object ever, as part of the API. However, that's often
+ quite a promise, given that this even prohibits object-internal caching or
+ lazy initialization of object variables. Moreover it's usually not too useful
+ for client applications. Hence: please be careful and avoid "const" on object
+ parameters, unless you are very sure "const" is appropriate.
+
+- Make sure to enforce limits on every user controllable resource. If the user
+ can allocate resources in your code, your code must enforce some form of
+ limits after which it will refuse operation. It's fine if it is hardcoded (at
+ least initially), but it needs to be there. This is particularly important
+ for objects that unprivileged users may allocate, but also matters for
+ everything else any user may allocated.
+
+- htonl()/ntohl() and htons()/ntohs() are weird. Please use htobe32() and
+ htobe16() instead, it's much more descriptive, and actually says what really
+ is happening, after all htonl() and htons() don't operation on longs and
+ shorts as their name would suggest, but on uint32_t and uint16_t. Also,
+ "network byte order" is just a weird name for "big endian", hence we might
+ want to call it "big endian" right-away.