Skip to content

dist/tools/coccinelle: add patch to fix asserts with side-effects#22200

Draft
maribu wants to merge 2 commits intoRIOT-OS:masterfrom
maribu:dist/tools/coccinelle/fix-assert
Draft

dist/tools/coccinelle: add patch to fix asserts with side-effects#22200
maribu wants to merge 2 commits intoRIOT-OS:masterfrom
maribu:dist/tools/coccinelle/fix-assert

Conversation

@maribu
Copy link
Copy Markdown
Member

@maribu maribu commented Apr 15, 2026

Contribution description

This adds a semantic patch to replace

assert(do_something() == 0);

with

typeof(do_something()) tmp = do_something();
(void)tmp;
assert(tmp == 0);

The reasoning that do_something() likely has side-effects and compilation with -DNDEBUG results in incorrect behavior.

A list of some 50 pure functions that safely can be used in assert() and are used in assert() has been added to disable all current false positives.

Testing procedure

Adding some assert(do_something()) should now result in the CI detecting the issue.

Issues/PRs references

None

Declaration of AI-Tools / LLMs usage:

AI-Tools / LLMs that were used are:

  • none

This adds a semantic patch to replace

```C
assert(do_something() == 0);
```

with

```C
typeof(do_something()) tmp = do_something();
(void)tmp;
assert(tmp == 0);
```

The reasoning that `do_something()` likely has side-effects and
compilation with `-DNDEBUG` results in incorrect behavior.

A list of some 50 pure functions that safely can be used in `assert()`
and are used in `assert()` has been added to disable all current false
positives.
@github-actions github-actions Bot added the Area: tools Area: Supplementary tools label Apr 15, 2026
@maribu maribu added CI: skip compile test If set, CI server will run only non-compile jobs, but no compile jobs or their dependent jobs Area: tools Area: Supplementary tools and removed Area: tools Area: Supplementary tools labels Apr 15, 2026
@maribu
Copy link
Copy Markdown
Member Author

maribu commented Apr 15, 2026

Not sure if we really want to merge this. It will force people adding some new assert(is_foo_valid()) to also add is_foo_valid to the list of currently ~50 pure functions used in assert()s.

On the other hand, it would prevent a certain flavor footguns from being merged.

@crasbe crasbe added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Apr 15, 2026
@riot-ci
Copy link
Copy Markdown

riot-ci commented Apr 15, 2026

Murdock results

✔️ PASSED

473513d squash! dist/tools/coccinelle: add patch to fix asserts with side-effects

Success Failures Total Runtime
1 0 1 01m:42s

Artifacts

Copy link
Copy Markdown
Contributor

@kfessel kfessel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assume should not be covert as its side effects are are effective regardless of NDEBUG

Comment thread dist/tools/coccinelle/force/assert_no_function_call.cocci Outdated
…ects

Co-authored-by: Karl Fessel <karl.fessel@ml-pa.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: tools Area: Supplementary tools CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: skip compile test If set, CI server will run only non-compile jobs, but no compile jobs or their dependent jobs Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants