sys/can/isotp: various style and robustness fixes

[maribu]

Contribution description

• validate received length indicators against actual CAN frame size
• consistently use size_t instead of a mix int/unsigned for array indices
• make the file IWYU clean
• fix some style issues

Testing procedure

No idea, I'm not really familiar with the code. Someone(TM) should do some testing before merging this.

Issues/PRs references

Missing validation of the received CAN frames reported by Shin, Sean J from the Georgia Institute of Technologies. Thanks for that!

Declaration of AI-Tools / LLMs usage:

AI-Tools / LLMs that were used are:

• none

[riot-ci]

Murdock results

✔️ PASSED

8734119 sys/can/isotp: various style and robustness fixes

Success	Failures	Total	Runtime
11108	0	11108	11m:39s

Artifacts

• Documentation preview

[kfessel]

ae is either 0 or 1 -> size_t covers that
indices in size_t ✔️

framesize might be unchecked in
_isotp_rcv(...) 414
and
_isotp_rcv_cf(...) 357

[kfessel]

looks right

did not test

[crasbe]

Please rebase, now that #22251 is merged :)

[maribu]

Someone(TM) really should make the standalone static test run optional. Murdock does a static test run as well that is mandatory to pass anyway.

[crasbe]

Someone(TM) really should make the standalone static test run optional. Murdock does a static test run as well that is mandatory to pass anyway.

But which ones are the "optional" ones that would not also fail on a merge to master?

Mostly buildsystem-sanity-check and perhaaaaaaps doccheck if someone is nice enough to fix issues in a different PR (like "undocumented blabla").

We could extend the static test script to accept some command line parameters to exclude buildsystem-sanity-check and run that as a separate step and allow that step to fail with continue-on-error https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax#jobsjob_idstepscontinue-on-error

Other than that I think the static test is nice to have.

[maribu]

I agree that it is nice to have the static test result early on. But making a failure a warning that maintainers may choose to ignore wouldn't be an issue, as the static-test is run a second time.

Another cause of static-tests failing is an update of codespell finding new typos. If there is still an old static-test result that passed from ages ago, everything is fine. But if you let the static-test run again, it will complain about typos already fixed in master.

The static-test run in Murdock will first rebase the PR on top of master. That way, it will only fail on issues that are freshly introduced by the PR.

[crasbe]

The static-test run in Murdock will first rebase the PR on top of master. That way, it will only fail on issues that are freshly introduced by the PR.

Okay but how about rebasing it in the workflow to the base branch of the PR? 🤔

[maribu]

Okay but how about rebasing it in the workflow to the base branch of the PR? 🤔

Yes, that be great.

[maribu]

Thx