28.1.2. Coding Style
Suricata uses a fairly strict coding style. This document describes it.
28.1.2.1. Formatting
28.1.2.1.1. clang-format
clang-format is configured to help you with formatting C code.
Note
The .clang-format script requires clang 9 or newer. At this
time clang-format-14 is used to validate formatting in CI.
28.1.2.1.1.1. Format your Changes
Before opening a pull request, please also try to ensure it is formatted
properly. We use clang-format for this, which has git integration through the
git-clang-format script to only format your changes.
On some systems, it may already be installed (or be installable via your package manager). If so, you can simply run it.
It is recommended to format each commit as you go. However, you can always reformat your whole branch after the fact.
Note
Depending on your installation, you might have to use the version-specific
git clang-format in the commands below, e.g. git clang-format-14,
and possibly even provide the clang-format binary with
--binary clang-format-14.
As an alternative, you can use the provided scripts/clang-format.sh
that isolates you from the different versions.
28.1.2.1.1.1.1. Formatting the most recent commit only
The following command will format only the code changed in the most recent commit:
$ git clang-format HEAD^
# Or with script:
$ scripts/clang-format.sh commit
Note that this modifies the files, but doesn't commit them. If the changes are trivial, you’ll likely want to run
$ git commit --amend -a
in order to update the last commit with all pending changes.
For bigger formatting changes, we do ask you to add them to separate, dedicated commits.
28.1.2.1.1.1.2. Formatting code in staging
The following command will format the changes in staging, i.e. files you
git add-ed:
$ git clang-format
# Or with script:
$ scripts/clang-format.sh cached
If you also want to change the unstaged changes, do:
$ git clang-format --force
# Or with script:
$ scripts/clang-format.sh cached --force
28.1.2.1.1.1.3. Formatting your branch's commits
In case you have multiple commits on your branch already and forgot to format them you can fix that up as well.
The following command will format every commit in your branch off master and rewrite history using the existing commit metadata.
Tip: Create a new version of your branch first and run this off the new version.
# In a new version of your pull request:
$ scripts/clang-format.sh rewrite-branch
Note that the above should only be used for rather minimal formatting changes. As mentioned, we prefer that you add such changes to a dedicated commit for formatting changes:
# Format all changes by commits on your branch:
$ git clang-format first_commit_on_your_branch^
# Or with script:
$ scripts/clang-format.sh branch
Note the usage of first_commit_on_your_branch^, not master, to avoid picking up
new commits on master in case you've updated master since you've branched.
28.1.2.1.1.1.4. Check formatting
Check if your branch changes' formatting is correct with:
$ scripts/clang-format.sh check-branch
Add the --diffstat parameter if you want to see the files needing formatting.
Add the --diff parameter if you want to see the actual diff of the formatting
change.
28.1.2.1.1.1.5. Formatting a whole file
Note Do not reformat whole files by default, i.e. do not use
|
If you were ever to do so, formatting changes of existing code with clang-format shall be a different commit and must not be mixed with actual code changes.
$ clang-format -i {file}
28.1.2.1.1.2. Disabling clang-format
There might be times, where the clang-format's formatting might not please. This might mostly happen with macros, arrays (single or multi-dimensional ones), struct initialization, or where one manually formatted code.
You can always disable clang-format.
/* clang-format off */
#define APP_LAYER_INCOMPLETE(c, n) (AppLayerResult){1, (c), (n)}
/* clang-format on */
28.1.2.1.1.3. Installing clang-format and git-clang-format
clang-format 9 or newer is required.
On Ubuntu 24.04:
It is sufficient to only install clang-format, e.g.
$ sudo apt-get install clang-format-14
See http://apt.llvm.org for other releases in case the clang-format version is not found in the default repos.
On Fedora:
Install the
clangandgit-clang-formatpackages with$ sudo dnf install clang git-clang-format
28.1.2.1.2. Line length
Limit line lengths to 100 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 100 character limit.
- clang-format:
ColumnLimit: 100
ContinuationIndentWidth: 8
ReflowComments: true
28.1.2.1.3. 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;
}
...
DecodeNetworkLayer(tv, dtv, SCNtohs(p->ethh->eth_type), p,
pkt + ETHERNET_HEADER_LEN, len - ETHERNET_HEADER_LEN);
return TM_ECODE_OK;
}
Use 8 space indentation when wrapping function parameters, loops and if statements.
Use 4 space indentation when wrapping variable definitions.
const SCPlugin PluginSpec = {
.name = OUTPUT_NAME,
.author = "Some Developer",
.license = "GPLv2",
.Init = TemplateInit,
};
28.1.2.1.4. Braces
Functions should have the opening brace on a newline:
int SomeFunction(void)
{
DoSomething();
}
Note: you may encounter non-compliant code.
Control and loop 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;
}
for (ascii_code = 0; ascii_code < 256; ascii_code++) {
ctx->goto_table[ctx->state_count][ascii_code] = SC_AC_FAIL;
}
while (funcs != NULL) {
temp = funcs;
funcs = funcs->next;
SCFree(temp);
}
Opening and closing braces go on the same line as as the _else_ (also known as a "cuddled else").
if (this) {
DoThis();
} else {
DoThat();
}
Structs, unions and enums should have the opening brace on the same line:
union {
TCPVars tcpvars;
ICMPV4Vars icmpv4vars;
ICMPV6Vars icmpv6vars;
} l4vars;
struct {
uint8_t type;
uint8_t code;
} icmp_s;
enum {
DETECT_TAG_TYPE_SESSION,
DETECT_TAG_TYPE_HOST,
DETECT_TAG_TYPE_MAX
};
- clang-format:
BreakBeforeBraces: Custom [breakbeforebraces]
BraceWrapping:
AfterClass: true
AfterControlStatement: false
AfterEnum: false
AfterFunction: true
AfterStruct: false
AfterUnion: false
AfterExternBlock: true
BeforeElse: false
IndentBraces: false
28.1.2.2. Flow
Don't use conditions and statements on the same line. E.g.
if (a) b = a; // <- wrong
if (a)
b = a; // <- right
for (int i = 0; i < 32; ++i) f(i); // <- wrong
for (int i = 0; i < 32; ++i)
f(i); // <- right
Don't put short or empty functions and structs on one line.
void empty_function(void)
{
}
int short_function(void)
{
return 1;
}
Don't use unnecessary branching. E.g.:
if (error) {
goto error;
} else {
a = b;
}
Can be written as:
if (error) {
goto error;
}
a = b;
- clang-format:
AllowShortBlocksOnASingleLine: false [llvm]
AllowShortBlocksOnASingleLine: Never [llvm] (breaking change in clang 10!) [clang10]
AllowShortEnumsOnASingleLine: false [clang11]
AllowShortFunctionsOnASingleLine: None
AllowShortIfStatementsOnASingleLine: Never [llvm]
AllowShortLoopsOnASingleLine: false [llvm]
BreakBeforeBraces: Custom [breakbeforebraces]
BraceWrapping:
SplitEmptyFunction: true
SplitEmptyRecord: true
28.1.2.3. Alignment
28.1.2.3.1. Pointers
Pointers shall be right aligned.
void *ptr;
void f(int *a, const char *b);
void (*foo)(int *);
- clang-format:
PointerAlignment: Right
DerivePointerAlignment: false
28.1.2.3.2. Declarations and Comments
Trailing comments should be aligned for consecutive lines.
struct bla {
int a; /* comment */
unsigned bb; /* comment */
int *ccc; /* comment */
};
void alignment()
{
// multiple consecutive vars
int a = 13; /* comment */
int32_t abc = 1312; /* comment */
int abcdefghikl = 13; /* comment */
}
- clang-format:
AlignConsecutiveAssignments: false
AlignConsecutiveDeclarations: false
AlignTrailingComments: true
28.1.2.4. Functions
28.1.2.4.1. parameter names
TODO
28.1.2.4.2. Function names
Function names are SCNamedLikeThis(). All non-static functions should be prefixed with SC.
static SCConfNode *SCConfGetNodeOrCreate(char *name, int final)
28.1.2.4.3. static vs non-static
Functions should be declared static whenever possible.
28.1.2.4.4. inline
The inlining of functions should be used only in critical paths.
28.1.2.5. Variables
28.1.2.5.1. Names
A variable is named_like_this in all lowercase.
SCConfNode *parent_node = root;
Generally, use descriptive variable names.
In loop vars, make sure i is a signed int type.
28.1.2.5.2. Scope
TODO
28.1.2.6. Macros
Macro names are ALL_CAPS_WITH_UNDERSCORES. Enclose parameters in parens on each usage inside the macro.
Align macro values on consecutive lines.
#define ACTION_ALERT 0x01
#define ACTION_DROP 0x02
#define ACTION_REJECT 0x04
#define ACTION_REJECT_DST 0x08
#define ACTION_REJECT_BOTH 0x10
#define ACTION_PASS 0x20
Align escape for multi-line macros right-most at ColumnLimit.
#define MULTILINE_DEF(a, b) \
if ((a) > 2) { \
auto temp = (b) / 2; \
(b) += 10; \
someFunctionCall((a), (b)); \
}
- clang-format:
AlignConsecutiveMacros: true [clang9]
AlignEscapedNewlines: Right
28.1.2.8. File names
File names are all lowercase and have a .c. .h or .rs (Rust) 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
28.1.2.9. Enums
Use a common prefix for all enum values. Value names are ALL_CAPS_WITH_UNDERSCORES.
Put each enum values on a separate line. Tip: Add a trailing comma to the last element to force "one-value-per-line" formatting in clang-format.
Enums exposed in a header file should be prefixed with SC_.
enum { VALUE_ONE, VALUE_TWO }; // <- wrong
// right
enum {
VALUE_ONE,
VALUE_TWO, // <- force one-value-per-line
};
- clang-format:
AllowShortEnumsOnASingleLine: false [clang11]
28.1.2.10. Structures and typedefs
Structures and typedefs use TitleCase naming. When exposed in a
header file they must be prefixed with SC.
For example:
typedef struct SCPlugin_ {
} SCPlugin;
28.1.2.11. 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;
Do not put short case labels on one line. Put opening brace on same line as case statement.
switch (a) {
case 13: {
int a = bla();
break;
}
case 15:
blu();
break;
default:
gugus();
}
- clang-format:
IndentCaseLabels: true
IndentCaseBlocks: false [clang11]
AllowShortCaseLabelsOnASingleLine: false [llvm]
BreakBeforeBraces: Custom [breakbeforebraces]
BraceWrapping:
AfterCaseLabel: false (default)
28.1.2.12. const
TODO
28.1.2.13. goto
Goto statements 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;
}
Put goto labels at brace level.
int goto_style_nested()
{
if (foo()) {
label1:
bar();
}
label2:
return 1;
}
- clang-format:
IndentGotoLabels: true (default) [clang10]
28.1.2.14. Includes
A .c file shall include it's own header first, or immediately after
suricata-common.h.
- clang-format:
SortIncludes: false
28.1.2.15. Unittests
When writing unittests that use 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,
28.1.2.16. Banned functions
function |
replacement |
reason |
|---|---|---|
strtok |
strtok_r |
|
sprintf |
snprintf |
unsafe |
strcat |
strlcat |
unsafe |
strcpy |
strlcpy |
unsafe |
strncpy |
strlcat |
|
strncat |
strlcpy |
|
strndup |
OS specific |
|
strchrnul |
||
rand |
||
rand_r |
||
index |
||
rindex |
||
bzero |
memset |
Also, check the existing code. If yours is wildly different, it's wrong. Example: https://github.com/oisf/suricata/blob/master/src/decode-ethernet.c
28.1.2.17. Rust
28.1.2.17.1. Pure Rust Code
Rust functions should follow normal Rust style where appropriate, for example:
pub fn try_new_array() -> Result<()> {
Ok(())
}
New Rust code should be formatted with rustfmt or cargo
fmt. If reformatting an existing file, format and commit before
making any changes. Such reformatting may be rejected in a PR based on
a variety of factors.
28.1.2.17.2. FFI
Rust code that is exposed to C should follow our C code style with
respect to naming. This applies to all functions marked as
#[no_mangle]. For example:
#[no_mangle]
pub extern "C" SCJbNewArray() -> *mut JsonBuilder {
}
Footnotes
28.1.2.7. Comments
28.1.2.7.1. Function comments
We use Doxygen, functions are documented using Doxygen notation:
28.1.2.7.2. General comments
We use
/* foobar */style and try to avoid//style.