Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable misra c2012 8.7 #1953

Closed
wants to merge 5 commits into from
Closed

Conversation

dzid26
Copy link
Contributor

@dzid26 dzid26 commented May 22, 2024

Bounty #1794
I picked up after #1830., rebased on the head and made minor fixes.

dzid26 added 2 commits May 22, 2024 13:23
Functions referenced in only one translation unit shall be static
Objects referenced in only one translation unit shall be in that unit and static
@dzid26 dzid26 marked this pull request as draft May 22, 2024 12:54
dzid26 added 2 commits May 22, 2024 18:55
If objects are not used, delete
If objects are referenced in one function, reduce its scope instead of making static - to avoid misra 8.9
@dzid26 dzid26 force-pushed the enable-misra-c2012-8.7 branch from afa97ea to 058d547 Compare May 22, 2024 19:37
@dzid26
Copy link
Contributor Author

dzid26 commented May 22, 2024

The PR fixes rule 8.7, but this rule seems pointless for the "hierarchical headers" architecture of Panda codebase

In the context of rule 8.7, using static is meant to reduce visibility within a given translation unit.
But Panda has only one translation unit (main.c) as it doesn't use the common source/header pair architecture, so it requires static in front of every function and object defined externally.

Cppcheck 2.13 doesn't detect all of them as violations (maybe it gets confused by the hierarchical includes). cppcheck 2.14 detects more, but not all off them. For example it detects dos_check_ignition() but not dos_read_som_gpio()

There is one benefit of solving 8.7 that I can see. - It allows to detect 8.9 violations. 8.9 is about narrowing scope of the objects - which is good. It sometimes makes the code look more messy though, when constants need to be moved from top of the file to a function body.

@dzid26 dzid26 force-pushed the enable-misra-c2012-8.7 branch from 6d5b879 to 3642625 Compare May 22, 2024 22:32
Functions referenced in only one translation unit shall be static
Delete unused functions.
@dzid26 dzid26 force-pushed the enable-misra-c2012-8.7 branch from 3642625 to 2c632c0 Compare May 22, 2024 22:43
@dzid26 dzid26 mentioned this pull request May 23, 2024
@dzid26 dzid26 closed this May 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant