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

Code Design

Most of the design is about getting as many compiler warnings and errors as possible. Issues found by static analyzers should also be fixed in some way, e.g. adding extra asserts. On 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.

Types

Function parameters

Function return values

Boolean expressions

Try to use boolean expressions the way they work in Java. C doesn't require this, 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). I'm planning on patching clang some day to give warnings when booleans aren't used properly.

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:

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.

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.