mirror of https://github.com/OISF/suricata
You cannot select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
335 lines
7.3 KiB
ReStructuredText
335 lines
7.3 KiB
ReStructuredText
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.
|
|
|
|
.. code-block:: c
|
|
|
|
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:
|
|
|
|
.. code-block:: c
|
|
|
|
int SomeFunction(void)
|
|
{
|
|
DoSomething();
|
|
}
|
|
|
|
|
|
Control statements should have the opening brace on the same line:
|
|
|
|
.. code-block:: c
|
|
|
|
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").
|
|
|
|
.. code-block:: c
|
|
|
|
if (this) {
|
|
DoThis();
|
|
} else {
|
|
DoThat();
|
|
}
|
|
|
|
Flow
|
|
~~~~
|
|
|
|
Don't use conditions and statements on the same line. E.g.
|
|
|
|
.. code-block:: c
|
|
|
|
if (a) b = a; // <- wrong
|
|
|
|
if (a)
|
|
b = a; // <- right
|
|
|
|
Don't use unnecessary branching. E.g.:
|
|
|
|
.. code-block:: c
|
|
|
|
if (error) {
|
|
goto error;
|
|
} else {
|
|
a = b;
|
|
}
|
|
|
|
|
|
Can be written as:
|
|
|
|
.. code-block:: c
|
|
|
|
if (error) {
|
|
goto error;
|
|
}
|
|
a = b;
|
|
|
|
Functions
|
|
~~~~~~~~~
|
|
|
|
parameter names
|
|
^^^^^^^^^^^^^^^
|
|
|
|
TODO
|
|
|
|
Function names
|
|
^^^^^^^^^^^^^^
|
|
|
|
Function names are NamedLikeThis().
|
|
|
|
.. code-block:: c
|
|
|
|
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:
|
|
|
|
.. code-block:: c
|
|
|
|
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.
|
|
|
|
.. code-block:: c
|
|
|
|
ConfNode *parent_node = root;
|
|
|
|
Generally, use descriptive variable names.
|
|
|
|
In loop vars, make sure ``i`` is a signed int type.
|
|
|
|
Scope
|
|
^^^^^
|
|
|
|
TODO
|
|
|
|
Macros
|
|
~~~~~~
|
|
|
|
TODO
|
|
|
|
Comments
|
|
~~~~~~~~
|
|
|
|
TODO
|
|
|
|
Function comments
|
|
^^^^^^^^^^^^^^^^^
|
|
|
|
We use Doxygen, functions are documented using Doxygen notation:
|
|
|
|
.. code-block:: c
|
|
|
|
/**
|
|
* \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. .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``
|
|
|
|
Enums
|
|
~~~~~
|
|
|
|
TODO
|
|
|
|
Structures and typedefs
|
|
~~~~~~~~~~~~~~~~~~~~~~~
|
|
|
|
TODO
|
|
|
|
switch statements
|
|
~~~~~~~~~~~~~~~~~
|
|
|
|
Switch statements are indented like in the following example, so the 'case' is indented from the switch:
|
|
|
|
.. code-block:: c
|
|
|
|
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.:
|
|
|
|
.. code-block:: c
|
|
|
|
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
|
|
~~~~~
|
|
|
|
TODO
|
|
|
|
goto
|
|
~~~~
|
|
|
|
Goto statements should be used with care. Generally, we use it primarily for error handling. E.g.:
|
|
|
|
.. code-block:: c
|
|
|
|
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:
|
|
|
|
.. code-block:: c
|
|
|
|
int SMTPProcessDataChunkTest02(void)
|
|
{
|
|
char mimemsg[] = {0x4D, 0x49, 0x4D, 0x45, 0x2D, 0x56, 0x65, 0x72,
|
|
|
|
you should have something like:
|
|
|
|
.. code-block:: c
|
|
|
|
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 | | |
|
|
+------------+---------------+-----------+
|
|
| 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
|