Skip to content

Commit ad00bc3

Browse files
committed
Add styling tool.
1 parent 8aa9807 commit ad00bc3

File tree

7 files changed

+36439
-18
lines changed

7 files changed

+36439
-18
lines changed

ProjectAnalysisReport.md

Lines changed: 114 additions & 18 deletions
Large diffs are not rendered by default.

clang-styling/README.md

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
# Overview
2+
3+
This directory contains scripts and the results of statical analysis tools applied to Zephyr OS's bluetooth host submodule. The goal of the analysis was to check this module's compliance with the `.clang-format` rules in the root of `zephyr/` directory and to uncover the other possible issues with the code.
4+
5+
## Style check
6+
7+
`run_style_check.sh` script is used to automatize `clang-format` runs as follows:
8+
9+
* All of the .c and .h files are found within the target directory (`subsys/bluetooth/host`).
10+
* `clang-format` is run in one of the following two modes:
11+
* Check mode: runs in `--dry-run` mode, only reporting the errors without fixing them. Violations are stored in a `.log` file.
12+
* Fix mode: runs with an `-i` option, fixing the violations in place.
13+
14+
### Results
15+
16+
There were no major violations reported, all of the 13k reported lines point towards the minor styling changes. Those were applied, and the the resulting changes are stored in a `host_clang_suggestions.diff` file.
17+
18+
## Clang-tidy analysis
19+
20+
The focus of the clang-tidy analysis is on the subsys/bluetooth/host/cs.c file. The goal is to identify possible errors, dangerous (bug-prone) coding patterns, and some optimization possibilities.
21+
22+
`run_tidy_check.sh` script is used to automatize `clang-tidy` runs. The script builds a sample application to generate a `compile_commands.json` file, which is necessary for the analysis, and then runs `clang-tidy`, filtering the output to show only warnings relevant to the target file (`cs.c`).
23+
24+
### Results
25+
26+
Analyzing the `cs.c` file, the `clang` tool found several issues with the code. Although no errors were found that would cause a system crash, warnings point towards the problems with maintainability, readability and potential bugs.
27+
28+
Below are the most important categories of issues found:
29+
30+
1) High cognitive complexity (`readability-function-cognitive-complexity`)
31+
Complexity is calculated incrementaly by noticing the confusing patterns within the function, such as nested ifs, complex ...
32+
* `alloc_reassembly_buf`: 60
33+
* `bt_hci_le_cs_read_remote_supported_capabilities_complete`: 128
34+
* `bt_hci_le_cs_read_remote_fae_table_complete`: 107
35+
Given that the threshold was set at 25, these functions are extremely difficult to understand, debug and modify. This can be considered the most serious risk found by the analysis.
36+
37+
2) Potential bugs
38+
* `bugprone-narrowing-conversions`
39+
`clang-tidy` detected "narrowing conversions," where a value of a wider type (e.g., `unsigned long`) is assigned to a variable of a narrower, signed type (`int16_t`).
40+
These types of conversions can lead to data loss or unexpected behavior if the original value cannot fit into the target type, which is a potential source of bugs.
41+
* `bugprone-easily-swappable-parameters`
42+
The function `bt_le_cs_get_antenna_path` has two adjacent parameters of the same type (`uint8_t`). It's possible (and plausible) to mistakenly swap these parameters when calling the function, and the compiler would not report an error, making this bug extremely hard to find.
43+
44+
3) Readability issues
45+
* `readability-magic-numbers`
46+
The code uses "magic numbers" (e.g., 12, 10, 0xFF) without a clear name or context.
47+
* `readability-identifier-length`
48+
The use of overly short variable names such as i, q, and cp. `clang-tidy` suggests that longer, more descriptive names improve the overall readability of the code.
49+
50+
Full report is stored within the `tidy_report.log` file.

0 commit comments

Comments
 (0)