HAProxy

HAProxy coding style for contributions

Mirror Sites: Master
Language: English

Introduction

A number of contributors are often embarrassed with coding style issues, they don't always know if they're doing it right, especially since the coding style has elvoved along the years. What is explained here is not necessarily what is applied in the code, but new code should as much as possible conform to this style. Coding style fixes happen when code is replaced. It is useless to send patches to fix coding style only, they will be rejected, unless they belong to a patch series which needs these fixes prior to get code changes. Also, please avoid fixing coding style in the same patches as functional changes, they make code review harder.

A good way to quickly validate your patch before submitting it is to pass it through the Linux kernel's checkpatch.pl utility which can be downloaded here :

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/plain/scripts/checkpatch.pl

Running it with the following options relaxes its checks to accommodate to the extra degree of freedom that is tolerated in HAProxy's coding style compared to the stricter style used in the kernel :

checkpatch.pl --ignore=LEADING_SPACE,CODE_INDENT,DEEP_INDENTATION,ELSE_AFTER_BRACE \
              -q --max-line-length=160 --no-tree --no-signoff < patch

You can take its output as hints instead of strict rules, but in general its output will be accurate and it may even spot some real bugs.

When modifying a file, you must accept the terms of the license of this file which is recalled at the top of the file, or is explained in the LICENSE file, or if not stated, defaults to LGPL version 2.1 or later for files in the include directory, and GPL version 2 or later for all other files.

When adding a new file, you must add a copyright banner at the top of the file with your real name, e-mail address and a reminder of the license. Contributions under incompatible licenses or too restrictive licenses might get rejected. If in doubt, please apply the principle above for existing files.

Tabs in this document will be represented as a series of 8 spaces so that it displays the same everywhere.

1) Indentation and alignment

1.1) Indentation

Indentation and alignment are two completely different things that people often get wrong. Indentation is used to mark a sub-level in the code. A sub-level means that a block is executed in the context of another block (eg: a function or a condition) :

main(int argc, char **argv)
{
        int i;

        if (argc < 2)
                exit(1);
}

In the example above, the code belongs to the main() function and the exit() call belongs to the if statement. Indentation is made with tabs (\t, ASCII 9), which allows any developer to configure their preferred editor to use their own tab size and to still get the text properly indented. Exactly one tab is used per sub-level. Tabs may only appear at the beginning of a line or after another tab. It is illegal to put a tab after some text, as it mangles displays in a different manner for different users (particularly when used to align comments or values after a #define). If you're tempted to put a tab after some text, then you're doing it wrong and you need alignment instead (see below).

Note that there are places where the code was not properly indented in the past. In order to view it correctly, you may have to set your tab size to 8 characters.

1.2) Alignment

Alignment is used to continue a line in a way to makes things easier to group together. By definition, alignment is character-based, so it uses spaces. Tabs would not work because for one tab there would not be as many characters on all displays. For instance, the arguments in a function declaration may be broken into multiple lines using alignment spaces :

int http_header_match2(const char *hdr, const char *end,
                       const char *name, int len)
{
...
}

In this example, the "const char *name" part is aligned with the first character of the group it belongs to (list of function arguments). Placing it here makes it obvious that it's one of the function's arguments. Multiple lines are easy to handle this way. This is very common with long conditions too :

        if ((len < eol - sol) &&
            (sol[len] == ':') &&
            (strncasecmp(sol, name, len) == 0)) {
                ctx->del = len;
        }

If we take again the example above marking tabs with "[-Tabs-]" and spaces with "#", we get this :

[-Tabs-]if ((len < eol - sol) &&
[-Tabs-]####(sol[len] == ':') &&
[-Tabs-]####(strncasecmp(sol, name, len) == 0)) {
[-Tabs-][-Tabs-]ctx->del = len;
[-Tabs-]}

It is worth noting that some editors tend to confuse indentations and aligment. Emacs is notoriously known for this brokenness, and is responsible for almost all of the alignment mess. The reason is that Emacs only counts spaces, tries to fill as many as possible with tabs and completes with spaces. Once you know it, you just have to be careful, as alignment is not used much, so generally it is just a matter of replacing the last tab with 8 spaces when this happens.

Indentation should be used everywhere there is a block or an opening brace. It is not possible to have two consecutive closing braces on the same column, it means that the innermost was not indented.

Right :

main(int argc, char **argv)
{
        if (argc > 1) {
                printf("Hello\n");
        }
        exit(0);
}

Wrong :

main(int argc, char **argv)
{
if (argc > 1) {
        printf("Hello\n");
}
exit(0);
}

A special case applies to switch/case statements. Due to my editor's settings, I've been used to align "case" with "switch" and to find it somewhat logical since each of the "case" statements opens a sublevel belonging to the "switch" statement. But indenting "case" after "switch" is accepted too. However in any case, whatever follows the "case" statement must be indented, whether or not it contains braces :

switch (*arg) {
case 'A': {
        int i;
        for (i = 0; i < 10; i++)
                printf("Please stop pressing 'A'!\n");
        break;
}
case 'B':
        printf("You pressed 'B'\n");
        break;
case 'C':
case 'D':
        printf("You pressed 'C' or 'D'\n");
        break;
default:
        printf("I don't know what you pressed\n");
}

2) Braces

Braces are used to delimit multiple-instruction blocks. In general it is preferred to avoid braces around single-instruction blocks as it reduces the number of lines :

Right :

if (argc >= 2)
        exit(0);

Wrong :

if (argc >= 2) {
        exit(0);
}

But it is not that strict, it really depends on the context. It happens from time to time that single-instruction blocks are enclosed within braces because it makes the code more symmetrical, or more readable. Example :

if (argc < 2) {
        printf("Missing argument\n");
        exit(1);
} else {
        exit(0);
}

Braces are always needed to declare a function. A function's opening brace must be placed at the beginning of the next line :

Right :

int main(int argc, char **argv)
{
        exit(0);
}

Wrong :

int main(int argc, char **argv) {
        exit(0);
}

Note that a large portion of the code still does not conforms to this rule, as it took years to get all authors to adapt to this more common standard which is now preferred, as it avoids visual confusion when function declarations are broken on multiple lines :

Right :

int foo(const char *hdr, const char *end,
        const char *name, const char *err,
        int len)
{
        int i;

Wrong :

int foo(const char *hdr, const char *end,
        const char *name, const char *err,
        int len) {
        int i;

Braces should always be used where there might be an ambiguity with the code later. The most common example is the stacked "if" statement where an "else" may be added later at the wrong place breaking the code, but it also happens with comments or long arguments in function calls. In general, if a block is more than one line long, it should use braces.

Dangerous code waiting of a victim :

if (argc < 2)
        /* ret must not be negative here */
        if (ret < 0)
                return -1;

Wrong change :

if (argc < 2)
        /* ret must not be negative here */
        if (ret < 0)
                return -1;
else
        return 0;

It will do this instead of what your eye seems to tell you :

if (argc < 2)
        /* ret must not be negative here */
        if (ret < 0)
                return -1;
        else
                return 0;

Right :

if (argc < 2) {
        /* ret must not be negative here */
        if (ret < 0)
                return -1;
}
else
        return 0;

Similarly dangerous example :

if (ret < 0)
        /* ret must not be negative here */
        complain();
init();

Wrong change to silent the annoying message :

if (ret < 0)
        /* ret must not be negative here */
        //complain();
init();

... which in fact means :

if (ret < 0)
        init();

3) Breaking lines

There is no strict rule for line breaking. Some files try to stick to the 80 column limit, but given that various people use various tab sizes, it does not make much sense. Also, code is sometimes easier to read with less lines, as it represents less surface on the screen (since each new line adds its tabs and spaces). The rule is to stick to the average line length of other lines. If you are working in a file which fits in 80 columns, try to keep this goal in mind. If you're in a function with 120-chars lines, there is no reason to add many short lines, so you can make longer lines.

In general, opening a new block should lead to a new line. Similarly, multiple instructions should be avoided on the same line. But some constructs make it more readable when those are perfectly aligned :

A copy-paste bug in the following construct will be easier to spot :

if (omult % idiv == 0) { omult /= idiv; idiv = 1; }
if (idiv % omult == 0) { idiv /= omult; omult = 1; }
if (imult % odiv == 0) { imult /= odiv; odiv = 1; }
if (odiv % imult == 0) { odiv /= imult; imult = 1; }

than in this one :

if (omult % idiv == 0) {
        omult /= idiv;
        idiv = 1;
}
if (idiv % omult == 0) {
        idiv /= omult;
        omult = 1;
}
if (imult % odiv == 0) {
        imult /= odiv;
        odiv = 1;
}
if (odiv % imult == 0) {
        odiv /= imult;
        imult = 1;
}

What is important is not to mix styles. For instance there is nothing wrong with having many one-line "case" statements as long as most of them are this short like below :

switch (*arg) {
case 'A': ret = 1; break;
case 'B': ret = 2; break;
case 'C': ret = 4; break;
case 'D': ret = 8; break;
default : ret = 0; break;
}

Otherwise, prefer to have the "case" statement on its own line as in the example in section 1.2 about alignment. In any case, avoid to stack multiple control statements on the same line, so that it will never be the needed to add two tab levels at once :

Right :

switch (*arg) {
case 'A':
        if (ret < 0)
                ret = 1;
        break;
default : ret = 0; break;
}

Wrong :

switch (*arg) {
case 'A': if (ret < 0)
                ret = 1;
        break;
default : ret = 0; break;
}

Right :

if (argc < 2)
        if (ret < 0)
                return -1;

or Right :

if (argc < 2)
        if (ret < 0) return -1;

but Wrong :

if (argc < 2) if (ret < 0) return -1;

When complex conditions or expressions are broken into multiple lines, please do ensure that alignment is perfectly appropriate, and group all main operators on the same side (which you're free to choose as long as it does not change for every block. Putting binary operators on the right side is preferred as it does not mangle with alignment but various people have their preferences.

Right :

if ((txn->flags & TX_NOT_FIRST) &&
    ((req->flags & BF_FULL) ||
     req->r < req->lr ||
     req->r > req->data + req->size - global.tune.maxrewrite)) {
        return 0;
}

Right :

if ((txn->flags & TX_NOT_FIRST)
    && ((req->flags & BF_FULL)
        || req->r < req->lr
        || req->r > req->data + req->size - global.tune.maxrewrite)) {
        return 0;
}

Wrong :

if ((txn->flags & TX_NOT_FIRST) &&
   ((req->flags & BF_FULL) ||
     req->r < req->lr
   || req->r > req->data + req->size - global.tune.maxrewrite)) {
        return 0;
}

If it makes the result more readable, parenthesis may even be closed on their own line in order to align with the opening one. Note that should normally not be needed because such code would be too complex to be digged into.

The "else" statement may either be merged with the closing "if" brace or lie on its own line. The later is preferred but it adds one extra line to each control block which is annoying in short ones. However, if the "else" is followed by an "if", then it should really be on its own line and the rest of the "if/else" blocks must follow the same style.

Right :

if (a < b) {
        return a;
}
else {
        return b;
}

Right :

if (a < b) {
        return a;
} else {
        return b;
}

Right :

HAProxy

HAProxy coding style for contributions

Quick links

Introduction

1) Indentation and alignment

1.1) Indentation

1.2) Alignment

2) Braces

3) Breaking lines