Writing code and testing it with unit and integration tests can provide high quality executable, however, it does not indicate if its a maintainable code, moreover code can have side effects when used outside its designed use, so in essence, tests pass, QA approved but then it gets in the field and crashes.
One of the ways to make sure code is maintainable, understandable and quick to get into is code reviews, however, using style checks and static code analysis can help automate some of it before a human can review it.
Applications are like Swiss cheese, one tool can verify certain aspects of it but many tools can discover more possible issues.
Code Style
Code style defines how a code looks like to the developer, it will most likely not affect how the code is run but it can affect how quickly a developer can understand what its doing.
clang-format
to install clang-format:
Once installed, we can generate a default clang-format configuration file and edit it if needed, clang-format defines a few standard styles: LLVM, GNU, Google, Chromium, Microsoft, Mozilla, WebKit
the following line will generate the configuration based on LLVM:
clang-format -style=LLVM -dump-config > .clang-format
For PlatformIO specific integration you can use my
runner and execute it as follows:
cpplint
cpplint is a tool that checks Google's C++ Style Guide, it can report the issues to command line or you can use the VSCode Extension and see the issues while writing code.
While the style guide is very strict, you can control how cpplint behaves by adjusting the linelength and which rule to apply or ignore in cpplint.cfg
For PlatformIO specific integration you can use my runner and execute it as follows:
By combining clang-format and cpplint you can avoid ever styling your code manually
Static Code Analysis
While coding style can really help for readability of code and preventing confusion, styling by itself does not contribute to the quality of code executed. Lets explore what is available Compiler Warnings
A great source of warnings and issues that originate from mistakes or lack of understanding of C/C++ languages is compiler warnings, the default warning level attempts to balance between certain mistakes and an attempt not to overwhelm the developer.
These flags can help you to switch between simple mistakes and pedantic development style, if you find yourself in a pickle or would like to avoid the pickle all together it might be beneficial to use the more restrictive warning levels but like everything in software, the tool does not make the software and you'll need to understand why you need to fix what the compiler tells you to fix.
Chris Coleman wrote about The Best and Worst GCC Compiler Flags For Embedded other than the GCC warnings documentation but in summary:
-Wall - enables warnings about questionable practices that are easy to avoid
-Wextra - more warnings
-Wshadow - shadowing is a readability issue that can also lead to bugs since the developer might get confused about which variable is actually in use.
-Wdouble-promotion - some MCUs have FPU that supports floats only, whenever a floating point gets promoted to double for any reason this warning will tell you about it since you might lose performance over it.
-Wformat=2 - checks scanf and printf mistakes
-Wformat-truncation - checks snprintf has enough room, heuristics based.
-Wundef - warning if undefined identifier is evaluated in the preprocessor.
-Weffc++ - Warn about violations of style guidelines from Scott Meyers’ Effective C++ series of books
In any case, you can always view which warnings are enabled by:
If you'd like to see which ones are enabled with using a certain warning level:
gcc -Wall -Wextra -Q --help=warnings
PlatformIO Check
PlatformIO
check provides easy access to two static code analyzers, cppcheck and clang-tidy. to use them we need to add:
check_tool = cppcheck, clangtidy
and then run:
This should get you results similar to this when running the checks:
Checking native > cppcheck (platform: native)
----------------------------------------------------------------------------------------------
src\port_arduino.h:6: [low:style] Function types shall be in prototype form with named parameters [misra-c2012-8.2]
...
================================ [PASSED] Took 3.53 seconds ==================================
Checking native > clangtidy (platform: native)
----------------------------------------------------------------------------------------------
src\main.cpp:6: [medium:warning] system include stdio.h not allowed [llvmlibc-restrict-system-libc-headers]
...
================================ [PASSED] Took 1.34 seconds ==================================
Component HIGH MEDIUM LOW
------------------ ------ -------- -----
lib\circularbuffer 0 0 32
...
Total 2 17 60
Environment Tool Status Duration
------------- --------- -------- ------------
native cppcheck PASSED 00:00:03.533
native clangtidy PASSED 00:00:01.341
================================= 2 succeeded in 00:00:04.874 ================================
cppcheck
cppcheck is a free static code analyzer, it detects common mistakes and also supports a subset of MISRA standard, you can find examples and explanations in the Zephyr documentation.
To support MISRA checks, you'll need to add a few things to the default PlatformIO configuration.
1. in platformio.ini section:
check_flags =
cppcheck: --enable=all --addon=./scripts/misra.json --addon=cert --addon=threadsafety --addon=y2038
2. download misra.py, misra_9.py and cppcheckdata.py from cppcheck repository and place it in scripts folder.
3. add misra.json to the scripts folder, this is the configuration for the MISRA addon, I've disabled rule 17.7 in this example.
{
"script": "scripts/misra.py",
"args": ["--rule-texts=scripts/misra.txt","--suppress-rules 17.7"]
}
4. download misra.txt and place it in the scripts folder for the addon to pick up and use as messages.
This should get you results similar to this when running the checks:
Checking native > cppcheck (platform: native)
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
src\port_arduino.h:6: [low:style] Function types shall be in prototype form with named parameters [misra-c2012-8.2]
src\port_arduino.h:7: [low:style] Function types shall be in prototype form with named parameters [misra-c2012-8.2]
src\port_arduino.h:10: [low:style] Function types shall be in prototype form with named parameters [misra-c2012-8.2]
lib\examplelib\ProductionCode.h:3: [low:style] Function types shall be in prototype form with named parameters [misra-c2012-8.2]
src\main.cpp:6: [low:style] The Standard Library input/output functions shall not be used [misra-c2012-21.6]
src\main.cpp:22: [low:style] Do not use the rand() function for generating pseudorandom numbers [cert-MSC30-c]
lib\circularbuffer\CircularBuffer.cpp:32: [low:style] There should be no unused parameters in functions [misra-c2012-2.7]
lib\circularbuffer\CircularBuffer.cpp:88: [low:style] A string literal shall not be assigned to an object unless the object's type is pointer to const-qualified char [misra-c2012-7.4]
lib\circularbuffer\CircularBuffer.cpp:102: [low:style] A string literal shall not be assigned to an object unless the object's type is pointer to const-qualified char [misra-c2012-7.4]
lib\circularbuffer\CircularBuffer.cpp:105: [low:style] A string literal shall not be assigned to an object unless the object's type is pointer to const-qualified char [misra-c2012-7.4]
lib\circularbuffer\CircularBuffer.h:50: [low:style] Function types shall be in prototype form with named parameters [misra-c2012-8.2]
lib\circularbuffer\CircularBuffer.h:52: [low:style] Function types shall be in prototype form with named parameters [misra-c2012-8.2]
lib\circularbuffer\CircularBuffer.h:53: [low:style] Function types shall be in prototype form with named parameters [misra-c2012-8.2]
lib\circularbuffer\CircularBuffer.h:54: [low:style] Function types shall be in prototype form with named parameters [misra-c2012-8.2]
clang-tidy
clang-tidy is llvm's static code analayzer, one of the more interesting features is that it can fix some errors it finds.
to run it with fix you can run it as follows:
pio check --flags "clangtidy: --fix"
This should get you results similar to this when running the checks:
Checking native > clangtidy (platform: native)
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
src\main.cpp:6: [medium:warning] system include stdio.h not allowed [llvmlibc-restrict-system-libc-headers]
src\main.cpp:6: [medium:warning] inclusion of deprecated C++ header 'stdio.h'; consider using 'cstdio' instead [hicpp-deprecated-headers,modernize-deprecated-headers]
src\main.cpp:9: [medium:warning] system include stdlib.h not allowed [llvmlibc-restrict-system-libc-headers]
src\main.cpp:9: [medium:warning] inclusion of deprecated C++ header 'stdlib.h'; consider using 'cstdlib' instead [hicpp-deprecated-headers,modernize-deprecated-headers]
src\main.cpp:16: [medium:warning] declaration must be declared within the '__llvm_libc' namespace [llvmlibc-implementation-in-namespace]
Flawfinder
Flawfinder is a simple tool for scanning source code for possible security weaknesses (or "flaws").
For PlatformIO specific integration you can use my
runner and execute it as follows:
This should get you results similar to this when running flawfinder:
flawfinder -C -c -D -i -S -Q include src lib\arduino-printf lib\circularbuffer lib\defectedLib lib\examplelib lib\runner
src\main.cpp:21:2: [0] (format) printf:If format strings can be influenced by an attacker, they can be exploited (CWE-134). Use a constant for the format specification. Constant format string, so not considered risky.
printf("test broken %d\r\n", FindFunction_WhichIsBroken(78));
src\main.cpp:24:2: [0] (format) printf:If format strings can be influenced by an attacker, they can be exploited (CWE-134). Use a constant for the format specification. Constant format string, so not considered risky.
printf("displaying float %.6f", c);
lib\circularbuffer\CircularBuffer.cpp:90:5: [2] (buffer) char:Statically-sized arrays can be improperly restricted, leading to potential overflows or other issues (CWE-119!/CWE-120). Perform bounds checking, use functions
that limit length, or ensure that the size is larger than the maximum possible length.
char sval[10];
lib\circularbuffer\CircularBuffer.cpp:98:9: [0] (format) snprintf:If format strings can be influenced by an attacker, they can be exploited, and note that sprintf variations do not always \0-terminate (CWE-134). Use a constant for the format specification. Constant format string, so not considered risky.
snprintf(sval, sizeof(sval), "%d", buffer[printIndex]);
lib\circularbuffer\CircularBuffer.cpp:90:5: [2] (buffer) char:Statically-sized arrays can be improperly restricted, leading to potential overflows or other issues (CWE-119!/CWE-120). Perform bounds checking, use functions
that limit length, or ensure that the size is larger than the maximum possible length.
char sval[10];
doxygen
while doxygen is not a static code analyzer per-se, it can help to generate documentation, call graphs and help other developers to understand the code in shorter time.
To ease with function documentation, you can use the Doxygen VSCode Extension.
Install doxygen and graphviz which is used to generate the graphs.
generate a basic configuration file:
As a quick start override the following settings:
OUTPUT_DIRECTORY = docs
INPUT = lib src
BUILTIN_STL_SUPPORT = YES
EXTRACT_ALL = YES
EXTRACT_STATIC = YES
WARN_NO_PARAMDOC = YES
RECURSIVE = YES
STRIP_CODE_COMMENTS = NO
REFERENCED_BY_RELATION = YES
REFERENCES_RELATION = YES
GENERATE_LATEX = NO
MACRO_EXPANSION = YES
HAVE_DOT = YES
UML_LOOK = YES
CALL_GRAPH = YES
CALLER_GRAPH = YES
INTERACTIVE_SVG = YES
Generate the documentation:
doxygen .doxygen
And finally, you should be able to see the documentation by opening the docs/html/index.html, search for functions and browse the documentation, you'll see that just after a few minutes you already know the basic structure, which function references which and you'll get a feeling of the general flow of the application.
Code Metrics
Code metrics are used to find hot spots that go against clean code's most prominent rule, “The first rule of functions is that they should be small.”
With code metrics we can find long functions, long files, complex functions and the most basic thing we can do to our team members to save them from this.
Lizard
First, we'll need to setup lizard's setting in platformio.ini, in this case we're limiting the complexity to 15, maximum length of functions to 100 lines and maximum arguments passed to a function is 1.
cyclomatic_complexity_analyzer = --CCN 15 --length 100 --arguments 1 --warning-msvs
For PlatformIO specific integration you can use my
runner and execute it as follows:
And see results similar to this:
lib\examplelib\ProductionCode2.c(4): warning: ThisFunctionHasNotBeenTested has 6 NLOC, 1 CCN, 28 token, 2 PARAM, 8 length
lizard -Eduplicate include src lib\arduino-printf lib\circularbuffer lib\defectedLib lib\examplelib lib\runner
*** Error 1
================================================
NLOC CCN token PARAM length location
------------------------------------------------
8 2 48 0 10 setup@16-25@src\main.cpp
3 1 5 0 4 loop@27-30@src\main.cpp
7 2 19 0 10 main@10-19@src\port_arduino.h
5 1 44 1 5 CircularBuffer::CircularBuffer@32-36@lib\circularbuffer\CircularBuffer.cpp
4 1 12 0 4 CircularBuffer::~CircularBuffer@38-41@lib\circularbuffer\CircularBuffer.cpp
4 1 10 0 4 CircularBuffer::IsEmpty@43-46@lib\circularbuffer\CircularBuffer.cpp
4 1 10 0 4 CircularBuffer::IsFull@48-51@lib\circularbuffer\CircularBuffer.cpp
8 3 49 1 8 CircularBuffer::Put@53-60@lib\circularbuffer\CircularBuffer.cpp
11 3 51 0 12 CircularBuffer::Get@62-73@lib\circularbuffer\CircularBuffer.cpp
4 1 10 0 4 CircularBuffer::Capacity@75-78@lib\circularbuffer\CircularBuffer.cpp
5 2 23 1 5 CircularBuffer::Next@80-84@lib\circularbuffer\CircularBuffer.cpp
18 5 116 0 22 CircularBuffer::Print@86-107@lib\circularbuffer\CircularBuffer.cpp
11 2 69 0 11 dynamic_buffer_overrun_018@5-15@lib\defectedLib\bufferLibrary.c
4 1 14 0 4 memory_leak_001@17-20@lib\defectedLib\bufferLibrary.c
9 3 36 1 9 FindFunction_WhichIsBroken@11-19@lib\examplelib\ProductionCode.c
4 1 9 1 4 FunctionWhichReturnsLocalVariable@21-24@lib\examplelib\ProductionCode.c
6 1 28 2 8 ThisFunctionHasNotBeenTested@4-11@lib\examplelib\ProductionCode2.c
3 1 11 0 3 setup@30-32@lib\runner\runner.h
13 file analyzed.
==============================================================
NLOC Avg.NLOC AvgCCN Avg.token function_cnt file
--------------------------------------------------------------
19 5.5 1.5 26.5 2 src\main.cpp
9 7.0 2.0 19.0 1 src\port_arduino.h
0 0.0 0.0 0.0 0 src\sdkconfig.h
1 0.0 0.0 0.0 0 lib\arduino-printf\arduino-printf.h
65 7.0 2.0 36.1 9 lib\circularbuffer\CircularBuffer.cpp
29 0.0 0.0 0.0 0 lib\circularbuffer\CircularBuffer.h
18 7.5 1.5 41.5 2 lib\defectedLib\bufferLibrary.c
2 0.0 0.0 0.0 0 lib\defectedLib\bufferLibrary.h
16 6.5 2.0 22.5 2 lib\examplelib\ProductionCode.c
2 0.0 0.0 0.0 0 lib\examplelib\ProductionCode.h
7 6.0 1.0 28.0 1 lib\examplelib\ProductionCode2.c
1 0.0 0.0 0.0 0 lib\examplelib\ProductionCode2.h
12 3.0 1.0 11.0 1 lib\runner\runner.h
===============================================================================================================
No thresholds exceeded (cyclomatic_complexity > 15 or length > 1000 or nloc > 1000000 or parameter_count > 100)
==========================================================================================
Total nloc Avg.NLOC AvgCCN Avg.token Fun Cnt Warning cnt Fun Rt nloc Rt
------------------------------------------------------------------------------------------
181 6.6 1.8 31.3 18 0 0.00 0.00
Duplicates
===================================
Total duplicate rate: 0.00%
Total unique rate: 100.00%
Now we can see a warning that a function has two parameters (over 1 of the limit we set)
We can also see statistics for the entire project analysis, this can help us locate functions that are approaching the limits we set and find duplicate code.
Coding Standards
In addition to MISRA, there are other interesting standards that you should be aware of.
SEI CERT C Coding Standard (PDF), what I really like about this standard is the explanation each rule have and why its in the standard.
Lastly but not less important is the AUTOSAR Guidelines for the use of the C++14 language in critical and safety-related systems (PDF), like the others, one of the more important sections is the rational for including the guidelines can provide an important insight into the "why" and is always a good reading material.
Test Sample
Lastly, if you're currently evaluating code standard tools and static code analysis tools, you may find the
itc-benchmarks beneficial.
References:
Evaluation of Open Source Static Analysis Security Testing (SAST) Tools for C
FOSS Static Analysis Tools for Embedded Systems and How to Use Them
Joint Strike Fighter Air Vehicle C++ Coding Standards