Style Guide

Overview of project files:

File Type	File Name/Extension	Description
Header	.hpp	Classes, enums, typedefs inside the icinga Namespace.
Source	.cpp	Method implementation for class functions, static/global variables.
CMake	CMakeLists.txt	Build configuration, source and header file references.
CMake Source	.cmake	Source/Header files generated from CMake placeholders.
ITL/conf.d	.conf	Template library and example files as configuration
Class Compiler	.ti	Object classes in our own language, generates source code as `<filename>-ti.{c,h}pp`.
Lexer/Parser	.ll, .yy	Flex/Bison code generated into source code from CMake builds.
Docs	.md	Markdown docs and READMEs.

Anything else are additional tools and scripts for developers and build systems.

All files must include the copyright header. We don’t use the current year as this implies yearly updates we don’t want.

Depending on the file type, this must be a comment.

/* Icinga 2 | (c) 2012 Icinga GmbH | GPLv2+ */

# Icinga 2 | (c) 2012 Icinga GmbH | GPLv2+

Code Formatting ¶

Tabs instead of spaces. Inside Visual Studio, choose to keep tabs instead of spaces. Tabs should use 4 spaces indent by default, depending on your likings.

We follow the clang format, with some exceptions.

Curly braces for functions and classes always start at a new line.

String ConfigObjectUtility::EscapeName(const String& name)
{
//...
}

String ConfigObjectUtility::CreateObjectConfig(const Type::Ptr& type, const String& fullName,
    bool ignoreOnError, const Array::Ptr& templates, const Dictionary::Ptr& attrs)
{
//...
}

Too long lines break at a parameter, the new line needs a tab indent.

    static String CreateObjectConfig(const Type::Ptr& type, const String& fullName,
        bool ignoreOnError, const Array::Ptr& templates, const Dictionary::Ptr& attrs);

Conditions require curly braces if it is not a single if with just one line.

    if (s == "OK") {
        //...
    } else {
        //...
    }

    if (!n)
        return;

There’s a space between if and the opening brace (. Also after the closing brace ) and opening curly brace {.
Negation with ! doesn’t need an extra space.
Else branches always start in the same line after the closing curly brace.

Code Comments ¶

Add comments wherever you think that another developer will have a hard time to understand the complex algorithm. Or you might have forgotten it in a year and struggle again. Also use comments to highlight specific stages in a function. Generally speaking, make things easier for the team and external contributors.

Comments can also be used to mark additional references and TODOs. If there is a specific GitHub issue or discussion going on, use that information as a summary and link over to it on purpose.

Single line comments may use // or /* ... */
Multi line comments must use this format:

/* Ensure to check for XY
 * This relies on the fact that ABC has been set before.
 */

Function Docs ¶

Function header documentation must be added. The current code basis needs rework, future functions must provide this.

Editors like CLion or Visual Studio allow you to type /** followed by Enter and generate the skeleton from the implemented function.

Add a short summary in the first line about the function’s purpose. Edit the param section with short description on their intention. The return value should describe the value type and additional details.

Example:

/**
 * Reads a message from the connected peer.
 *
 * @param stream ASIO TLS Stream
 * @param yc Yield Context for ASIO
 * @param maxMessageLength maximum size of bytes read.
 *
 * @return A JSON string
 */
String JsonRpc::ReadMessage(const std::shared_ptr<AsioTlsStream>& stream, boost::asio::yield_context yc, ssize_t maxMessageLength)

While we can generate code docs from it, the main idea behind it is to provide on-point docs to fully understand all parameters and the function’s purpose in the same spot.

Only include other headers which are mandatory for the header definitions. If the source file requires additional headers, add them there to avoid include loops.

The included header order is important.

First, include the library header i2-<libraryname>.hpp, e.g. i2-base.hpp.
Second, include all headers from Icinga itself, e.g. remote/apilistener.hpp. base before icinga before remote, etc.
Third, include third-party and external library headers, e.g. openssl and boost.
Fourth, include STL headers.

Source ¶

The included header order is important.

First, include the header whose methods are implemented.
Second, include all headers from Icinga itself, e.g. remote/apilistener.hpp. base before icinga before remote, etc.
Third, include third-party and external library headers, e.g. openssl and boost.
Fourth, include STL headers.

Always use an empty line after the header include parts.

Namespace ¶

The icinga namespace is used globally, as otherwise we would need to write icinga::Utility::FormatDateTime().

using namespace icinga;

Other namespaces must be declared in the scope they are used. Typically this is inside the function where boost::asio and variants would complicate the code.

    namespace ssl = boost::asio::ssl;

    auto context (std::make_shared<ssl::context>(ssl::context::sslv23));

Functions ¶

Ensure to pass values and pointers as const reference. By default, all values will be copied into the function scope, and we want to avoid this wherever possible.

std::vector<EventQueue::Ptr> EventQueue::GetQueuesForType(const String& type)

C++ only allows to return a single value. This can be abstracted with returning a specific class object, or with using a map/set. Array and Dictionary objects increase the memory footprint, use them only where needed.

A common use case for Icinga value types is where a function can return different values - an object, an array, a boolean, etc. This happens in the inner parts of the config compiler expressions, or config validation.

The function caller is responsible to determine the correct value type and handle possible errors.

Specific algorithms may require to populate a list, which can be passed by reference to the function. The inner function can then append values. Do not use a global shared resource here, unless this is locked by the caller.

Conditions and Cases ¶

Prefer if-else-if-else branches. When integers are involved, switch-case statements increase readability. Don’t forget about break though!

Avoid using ternary operators where possible. Putting a condition after an assignment complicates reading the source. The compiler optimizes this anyways.

Wrong:

    int res = s == "OK" ? 0 : s == "WARNING" ? 1;

    return res;

Better:

    int res = 3;

    if (s == "OK") {
        res = 0;
    } else if (s == "WARNING") {
        res = 1;
    }

Even better: Create a lookup map instead of if branches. The complexity is reduced to O(log(n)).

    std::map<String, unsigned int> stateMap = {
        { "OK", 1 },
        { "WARNING", 2 }
    }

    auto it = stateMap.find(s);

    if (it == stateMap.end()) {
        return 3
    }

    return it.second;

The code is not as short as with a ternary operator, but one can re-use this design pattern for other generic definitions with e.g. moving the lookup into a utility class.

Once a unit test is written, everything works as expected in the future.

Locks and Guards ¶

Lock access to resources where multiple threads can read and write. Icinga objects can be locked with the ObjectLock class.

Object locks and guards must be limited to the scope where they are needed. Otherwise we could create dead locks.

    {
        ObjectLock olock(frame.Locals);
        for (const Dictionary::Pair& kv : frame.Locals) {
            AddSuggestion(matches, word, kv.first);
        }
    }

Objects and Pointers ¶

Use shared pointers for objects. Icinga objects implement the Ptr typedef returning an intrusive_ptr for the class object (object.hpp). This also ensures reference counting for the object’s lifetime.

Use raw pointers with care!

Some methods and classes require specific shared pointers, especially when interacting with the Boost library.

Value Types ¶

Icinga has its own value types. These provide methods to allow generic serialization into JSON for example, and other type methods which are made available in the DSL too.

Always use String instead of std::string. If you need a C-string, use the CStr() method.
Avoid casts and rather use the Convert class methods.

    double s = static_cast<double>(v); //Wrong

    double s = Convert::ToDouble(v);   //Correct, ToDouble also provides overloads with different value types

Prefer STL containers for internal non-user interfaces. Icinga value types add a small overhead which may decrease performance if e.g. the function is called 100k times.
Array::FromVector and variants implement conversions, use them.

Utilities ¶

Don’t re-invent the wheel. The Utility class provides many helper functions which allow you e.g. to format unix timestamps, search in filesystem paths.

Also inspect the Icinga objects, they also provide helper functions for formatting, splitting strings, joining arrays into strings, etc.

Libraries ¶

2.11 depends on Boost 1.66. Use the existing libraries and header-only includes for this specific version.

Note: Prefer C++11 features where possible, e.g. std::atomic and lambda functions.

General:

exception (header only)
algorithm (header only)
lexical_cast (header only)
regex
uuid (header only)
range (header only)
variant (header only)
multi_index (header only)
function_types (header only)
circular_buffer (header only)
math (header only)

Events and Runtime:

Network I/O:

asio (header only)
beast (header only)
coroutine
context

Consider abstracting their usage into *utility.{c,h}pp files with wrapping existing Icinga types. That also allows later changes without rewriting large code parts.

Note

A new Boost library should be explained in a PR and discussed with the team.

This requires package dependency changes.

If you consider an external library or code to be included with Icinga, the following requirements must be fulfilled:

License is compatible with GPLv2+. Boost license, MIT works, Apache is not.
C++11 is supported, C++14 or later doesn’t work
Header only implementations are preferred, external libraries require packages on every distribution.
No additional frameworks, Boost is the only allowed.
The code is proven to be robust and the GitHub repository is alive, or has 1k+ stars. Good libraries also provide a user list, if e.g. Ceph is using it, this is a good candidate.

Log ¶

Icinga allows the user to configure logging backends, e.g. syslog or file.

Any log message inside the code must use the Log() function.

The first parameter is the severity level, use them with care.
The second parameter defines the location/scope where the log happened. Typically we use the class name here, to better analyse the logs the user provide in GitHub issues and on the community channels.
The third parameter takes a log message string

If the message string needs to be computed from existing values, everything must be converted to the String type beforehand. This conversion for every value is very expensive which is why we try to avoid it.

Instead, use Log() with the shift operator where everything is written on the stream and conversions are explicitly done with templates in the background.

The trick here is that the Log object is destroyed immediately after being constructed once. The destructor actually evaluates the values and sends it to registers loggers.

Since flushing the stream every time a log entry occurs is very expensive, a timer takes care of flushing the stream every second.

Tip

If logging stopped, the flush timer thread may be dead. Inspect that with gdb/lldb.

Avoid log messages which could irritate the user. During implementation, developers can change log levels to better see what’s going one, but remember to change this back to debug or remove it entirely.

Goto ¶

Avoid using goto statements. There are rare occasions where they are allowed:

The code would become overly complicated within nested loops and conditions.
Event processing and C interfaces.
Question/Answer loops within interactive CLI commands.

Typedef and Auto Keywords ¶

Typedefs allow developers to use shorter names for specific types, classes and structs.

    typedef std::map<String, std::shared_ptr<NamespaceValue> >::iterator Iterator;

These typedefs should be part of the Class definition in the header, or may be defined in the source scope where they are needed.

Avoid declaring global typedefs, unless necessary.

Using the auto keyword allows to ignore a specific value type. This comes in handy with maps/sets where no specific access is required.

The following example iterates over a map returned from GetTypes().

    for (const auto& kv : GetTypes()) {
        result.insert(kv.second);
    }

The long example would require us to define a map iterator, and a slightly different algorithm.

    typedef std::map<String, DbType::Ptr> TypeMap;
    typedef std::map<String, DbType::Ptr>::const_iterator TypeMapIterator;

    TypeMap types = GetTypes();

    for (TypeMapIterator it = types.begin(); it != types.end(); it++) {
        result.insert(it.second);
    }

We could also use a pair here, but requiring to know the specific types of the map keys and values.

    typedef std::pair<String, DbType::Ptr> kv_pair;

    for (const kv_pair& kv : GetTypes()) {
        result.insert(kv.second);
    }

After all, auto shortens the code and one does not always need to know about the specific types. Function documentation for GetTypes() is required though.

Whitespace Cleanup ¶

Patches must be cleaned up and follow the indent style (tabs instead of spaces). You should also remove any trailing whitespaces.

git diff allows to highlight such.

vim $HOME/.gitconfig

[color "diff"]
        whitespace = red reverse
[core]
        whitespace=fix,-indent-with-non-tab,trailing-space,cr-at-eol

vim also can match these and visually alert you to remove them.

vim $HOME/.vimrc

highlight ExtraWhitespace ctermbg=red guibg=red
match ExtraWhitespace /\s\+$/
autocmd BufWinEnter * match ExtraWhitespace /\s\+$/
autocmd InsertEnter * match ExtraWhitespace /\s\+\%#\@<!$/
autocmd InsertLeave * match ExtraWhitespace /\s\+$/
autocmd BufWinLeave * call clearmatches()