This documentation is for Dovecot v2.x, see wiki1 for v1.x documentation.

Code Design

Generally Dovecot follows Linux kernel coding style.

Most of the coding style design is about getting as many compiler warnings and errors as possible. Dovecot already has some patched-Clang-specific features to get more warnings, and will likely have more in future. Issues found by static analyzers should also be fixed in some way, e.g. adding extra asserts.

Runtime errors should be caught by asserts or NULL pointer dereferences, which cleanly crash the program instead of it continuing and possibly corrupting data or causing other bad things. Dovecot's master process restarts the crashed processes anyway. Obviously the master process should be very careful to avoid crashing, but even then in some OSes a crashed Dovecot master gets restarted by the init process.

Dovecot's bottlenecks are primarily disk I/O and secondarily memory usage, so using extra CPU for asserts and other extra checks costs practically nothing.

Types

Function parameters

Function return values

Boolean expressions

Try to use boolean expressions the way they work in Java. C doesn't require this, but I think it makes the code easier to understand and reduces bugs in some cases (e.g. if (!foo()) when thinking foo() returns bool/FALSE, but actually returns int/-1 on error). We've a clang patch to give warnings in these cases. As expected, it found quite a lot of bugs (some real bugs and a lot of "it just accidentally worked").

So:

Memory

Memory is always allocated through one of Dovecot's wrappers, e.g. i_malloc() or i_new(). All of Dovecot's memory allocations always succeed or kill the process. There's no point in writing a lot of code to check for memory allocation failures that happen just about never. The only reason some memory allocations fail in Dovecot currently is because a process VSZ limit is reached, which usually indicates either a memory leak or trying to access a mailbox that is too large. In either of these cases it's better to just completely restart the process than try to limp along without getting anything useful done anymore.

Memory allocations can be assumed to be zero-initialized. All of the memory allocation functions do it, except t_malloc() and t_buffer_get(), which you should almost never use directly anyway. The code currently also assumes that pointers in zero-initialized memory area are NULL, which isn't guaranteed by ANSI-C, but practically Dovecot isn't going to be run in systems where it's not true and you're not going to remember to NULL-initialize all of your pointers anyway without compiler/runtime failure.

When using a struct, always zero-initialize it with memset() instead of setting each field separately to 0. It's too easy to cause bugs by adding a new field to the struct and forgetting to initialize it.

Double-frees in C are bad. Better to crash with NULL pointer dereference than possibly allow an attacker to exploit heap corruption and run executable code. Most of the pointers are set to NULL when they are freed:

Buffers

Use dynamically growing strings/buffers wherever necessary instead of a static sized buffer, where on larger input the function fails or truncates the data. It's of course not good to allow users to infinitely grow memory usage, so there should be some limits added, but it shouldn't fail even if the limit is set to infinite.

Avoid explicitly calculating memory usage for allocations. If you do, mark it with /* @UNSAFE */ comment unless it's the calculation is "so obvious that you see it's correct at the first glance". If in doubt, just mark it UNSAFE. The idea is that anyone can easily grep for these and verify their correctness.

Type safety

It's better if compiler can give a warning when something is accidentally used wrong.

Callback functions

Callback functions make the code more difficult to follow, especially when a callback calls another callback, or when using function pointers to jump to different callbacks depending on state. Of course with asynchronous C code it's pretty much impossible to avoid callbacks. Still, try to avoid them where possible to keep the code readable. lib-fs/fs-api.h is an example API which supports async operations but with a single common "do more now" callback rather than every single operation having its own callback parameter. This makes it similar to async network IO with read()/write() EAGAIN handling.

Often callback functions can be avoided by creating iterator functions instead. For example instead of parse(callback, context use ctx = parse_init(); while (parse_next(ctx)) { .. } parse_deinit(&ctx);

Dovecot has some helper macros to make callbacks' context parameters type-safe. In v2.2+ see CALLBACK_TYPECHECK() macro and for example io_add() for example usage.

Design/Code (last edited 2016-12-11 16:28:58 by TimoSirainen)