... | ... | @@ -1161,7 +1161,7 @@ bool app__is_id_ready(const *module_s, const size_t type_id); // Bad, const type |
|
|
|
|
|
Assuming there is a heavy use of pointers, we want to balance the fundamental checks with those that may actually be useful and create a safer code-base.
|
|
|
|
|
|
We design code modules that are based on `structs` and the instances of these structs are then given to APIs. This mimics how C++ classes operate as the first parameter to functions is the module pointer itself which C++ refers to as the `this pointer`.
|
|
|
C code modules are designed based on `structs` and the instances of these structs are then given to APIs. This mimics how C++ classes operate as the first parameter to functions is the module pointer itself which C++ refers to as the `this pointer`.
|
|
|
|
|
|
```c
|
|
|
typedef struct {
|
... | ... | @@ -1172,7 +1172,7 @@ void my_module__api(my_module_s *this_ptr); |
|
|
```
|
|
|
|
|
|
* Validate and check all pointers being passed into public API
|
|
|
* We used to avoid this, but had to revert our decision based on safety certified code requirements
|
|
|
* We used to avoid this, but had to revert our decision based on safety certified code requirements. It introduced extra brace level and increased cyclomatic complexity, but it was necessary, and hence the next bullet point is striked-out.
|
|
|
* ~~Do not check the `this` pointer equivalent of C++ with `NULL`. This creates a lot of fluff, creates potential waste that will only be rarely useful because it is a programming error to pass `NULL` pointers and we cannot be over-protective about this condition.~~
|
|
|
* Private functions of the module can omit additional pointer checks as long as you can ensure through unit-testing that those pointers could never be bad memory pointers
|
|
|
* The rest of the pointers **should** be checked and **design code such that error conditions will not end up in a CPU exception**.
|
... | ... | @@ -1193,6 +1193,43 @@ void my_api(module_s *this_ptr, void *ptr) { |
|
|
}
|
|
|
```
|
|
|
|
|
|
### Pointer Validation Recommendation
|
|
|
|
|
|
Each pointer must be validated, and there are certain rules to follow in order to get past static analysis tools flagging your code, and preventing it from getting merged.
|
|
|
|
|
|
```c
|
|
|
// DO NOT DO THIS
|
|
|
// CERT C will flag this as 'warning 613: potential use of null pointer
|
|
|
void foo(module_s *module, void *pointer) {
|
|
|
const bool pointers_are_valid = (NULL != module) && (NULL != pointer);
|
|
|
|
|
|
if (pointers_are_valid) {
|
|
|
// ...
|
|
|
}
|
|
|
}
|
|
|
|
|
|
// Explicitly check pointers
|
|
|
void foo(module_s *module, void *pointer) {
|
|
|
if ((NULL != module) && (NULL != pointer)) {
|
|
|
// ...
|
|
|
}
|
|
|
}
|
|
|
```
|
|
|
|
|
|
If there are "private" (or static) functions, they do not need redundant checks as long as code structure guarantee that they will never be called unsafely. These are "forgiven" by PCLint or CERTC as long as they follow the coding standards of containing `__private_` name. Here is an example:
|
|
|
|
|
|
```c
|
|
|
static void module__private_do(module_s *module, void *pointer) {
|
|
|
// No need to check 'if ((NULL != module) && (NULL != pointer))'
|
|
|
}
|
|
|
|
|
|
void module__do(module_s *module, void *pointer) {
|
|
|
if ((NULL != module) && (NULL != pointer)) {
|
|
|
module__private_do(module, pointer);
|
|
|
}
|
|
|
}
|
|
|
```
|
|
|
|
|
|
## Variables
|
|
|
|
|
|
Do not re-use variables, and avoid changing the variables passed as copy. You can opt to make the parameters `const` to enforce this.
|
... | ... | |