Coding Style¶

Suricata uses a fairly strict coding style. This document describes it.

Formatting¶

Line length¶

There is a soft limit of ~80 characters.

When wrapping lines that are too long, they should be indented at least 8 spaces from previous line. You should attempt to wrap the minimal portion of the line to meet the 80 character limit.

Indent¶

We use 4 space indentation.

int DecodeEthernet(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p,
    uint8_t *pkt, uint16_t len, PacketQueue *pq)
{
    SCPerfCounterIncr(dtv->counter_eth, tv->sc_perf_pca);

    if (unlikely(len < ETHERNET_HEADER_LEN)) {
        ENGINE_SET_INVALID_EVENT(p, ETHERNET_PKT_TOO_SMALL);
        return TM_ECODE_FAILED;
    }

Braces¶

Functions should have the opening brace on a newline:

int SomeFunction(void)
{
    DoSomething();
}

Control statements should have the opening brace on the same line:

if (unlikely(len < ETHERNET_HEADER_LEN)) {
    ENGINE_SET_INVALID_EVENT(p, ETHERNET_PKT_TOO_SMALL);
    return TM_ECODE_FAILED;
}

Opening and closing braces go on the same line as as the else (also known as a "cuddled else").

if (this) {
    DoThis();
} else {
    DoThat();
}

Flow¶

Don't use conditions and statements on the same line. E.g.

if (a) b = a; // <- wrong

if (a)
    b = a; // <- right

Don't use unnecessary branching. E.g.:

    if (error) {
        goto error;
    } else {
        a = b;
    }

Can be written as:

    if (error) {
        goto error;
    }
    a = b;

Functions¶

Function names¶

Function names are NamedLikeThis().

static ConfNode *ConfGetNodeOrCreate(char *name, int final)

static vs non-static¶

Functions should be declared static whenever possible.

inline¶

The inlining of functions should be used only in critical paths.

curly braces / brackets¶

Functions should have the opening bracket on a newline:

int SomeFunction(void)
{
    DoSomething();
}

Note: this is a fairly new requirement, so you'll encounter a lot of non-compliant code.

Variables¶

Names¶

A variable is named_like_this in all lowercase.

    ConfNode *parent_node = root;

Generally, use descriptive variable names.

In loop vars, make sure i is and signed int.

Scope¶

Macro's¶

Comments¶

Function comments¶

We use Doxygen, functions are documented using Doxygen notation:

/**
 * \brief Helper function to get a node, creating it if it does not
 * exist.
 *
 * This function exits on memory failure as creating configuration
 * nodes is usually part of application initialization.
 *
 * \param name The name of the configuration node to get.
 * \param final Flag to set created nodes as final or not.
 *
 * \retval The existing configuration node if it exists, or a newly
 * created node for the provided name. On error, NULL will be returned.
 */
static ConfNode *ConfGetNodeOrCreate(char *name, int final)

General comments¶

We use /* foobar */ style and try to avoid // style.

File names¶

File names are all lowercase and have a .c or .h extension.

Most files have a subsystem prefix, e.g. detect-dsize.c, util-ip.c

Some cases have a multi-layer prefix, e.g. util-mpm-ac.c

Enums¶

Structures and typedefs¶

switch statements¶

Switch statements are indented like in the following example, so the 'case' is indented from the switch:

    switch (ntohs(p->ethh->eth_type)) {
        case ETHERNET_TYPE_IP:
            DecodeIPV4(tv, dtv, p, pkt + ETHERNET_HEADER_LEN,
                       len - ETHERNET_HEADER_LEN, pq);
            break;

Fall through cases will be commented with /* fall through */. E.g.:

        switch (suri->run_mode) {
            case RUNMODE_PCAP_DEV:
            case RUNMODE_AFP_DEV:
            case RUNMODE_PFRING:
                /* find payload for interface and use it */
                default_packet_size = GetIfaceMaxPacketSize(suri->pcap_dev);
                if (default_packet_size)
                    break;
                /* fall through */
            default:
                default_packet_size = DEFAULT_PACKET_SIZE;

const¶

goto¶

Goto's should be used with care. Generally, we use it primarily for error handling. E.g.:

static DetectFileextData *DetectFileextParse (char *str)
{
    DetectFileextData *fileext = NULL;

    fileext = SCMalloc(sizeof(DetectFileextData));
    if (unlikely(fileext == NULL))
        goto error;

    memset(fileext, 0x00, sizeof(DetectFileextData));

    if (DetectContentDataParse("fileext", str, &fileext->ext, &fileext->len, &fileext->flags) == -1) {
        goto error;
    }

    return fileext;

error:
    if (fileext != NULL)
        DetectFileextFree(fileext);
    return NULL;
}

Unittests¶

When writing unittests that use when using a data array containing a protocol message, please put an explanatory comment that contain the readable content of the message

So instead of:

int SMTPProcessDataChunkTest02(void)
{
    char mimemsg[] = {0x4D, 0x49, 0x4D, 0x45, 0x2D, 0x56, 0x65, 0x72,

you should have something like:

int SMTPParserTest14(void)
{
    /* 220 mx.google.com ESMTP d15sm986283wfl.6<CR><LF> */
    static uint8_t welcome_reply[] = { 0x32, 0x32, 0x30, 0x20,

Banned functions¶

function	replacement	reason
strok	strtok_r
sprintf	snprintf	unsafe
strcat	strlcat	unsafe
strcpy	strlcpy	unsafe
strncpy	strlcat
strncat	strlcpy
strndup		OS specific
strchrnul