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

Finish MISRA coverage #1794

Closed
adeebshihadeh opened this issue Jan 13, 2024 · 8 comments
Closed

Finish MISRA coverage #1794

adeebshihadeh opened this issue Jan 13, 2024 · 8 comments

Comments

@adeebshihadeh
Copy link
Contributor

adeebshihadeh commented Jan 13, 2024

In #926, we updated cppcheck to the latest version which completed cppcheck's MISRA coverage. These new checks were suppressed in that PR in order to get the cppcheck version updated, and now we want to enable those checks and fix the violations.

Read about MISRA C:2012 here; the guidelines and information about them can be found searching online (ChatGPT also knows about it).

$50 for each rule addressed, either fixed all violations or suppressed for good reason. There's 28 new rules to enable, and progress can be checked here.

  • each rule should get its own PR
  • PR title should be something like "enable misra-c2012-2.2"

Example for misra-c2012-2.2: f64ba24.

@adeebshihadeh
Copy link
Contributor Author

@0x41head @samrum @bongbui321 I missed these in my initial count, but the ones that don't have "misra" in the name also count:

unusedFunction
constParameterPointer
constParameterCallback
knownConditionTrueFalse

@0x41head
Copy link
Contributor

An update on unusedFunction :

Currently unusedFunction needs to see the whole code to avoid false positives making it unfeasible to enable that check on single files only. 

In the case of pandas, since we are only checking a couple of files, the cppchecker doesn't get the entire context of the codebase.
I confirmed this by running cpp checker over the entire codebase, where it gave no issues with respect to unusedFunction

Reference

@adeebshihadeh
Copy link
Contributor Author

In the case of pandas, since we are only checking a couple of files, the cppchecker doesn't get the entire context of the codebase. I confirmed this by running cpp checker over the entire codebase, where it gave no issues with respect to unusedFunction

Reference

Want to open a PR to run on the whole repo? Any reason not to do this?

@adeebshihadeh
Copy link
Contributor Author

10.3 seems to have false positives for bools. Putting up an extra $150 (for $200 total on that rule) to fix it in cppcheck and make an upstream PR to cppcheck. We can potentially ship the patch here first if they don't merge it quickly.

https://sourceforge.net/p/cppcheck/discussion/development/thread/c96d6d44f1/

@0x41head
Copy link
Contributor

0x41head commented Feb 4, 2024

Want to open a PR to run on the whole repo? Any reason not to do this?

Running over the whole repo would require a lot of time. Didn't really think the benefits justified the extra time required.

@adeebshihadeh
Copy link
Contributor Author

adeebshihadeh commented Feb 17, 2024

@0x41head I got the unused check running fast, but there are a few false positives (#1875). Bounty is still up for grabs if you can figure those out and get unusedFunction actually enabled.

https://github.com/commaai/panda/actions/runs/7944491038/job/21690064166

/tmp/openpilot/panda/board/drivers/pwm.h:5:0: style: The function 'pwm_init' is never used. [unusedFunction]
void pwm_init(TIM_TypeDef *TIM, uint8_t channel){
^
/tmp/openpilot/panda/board/drivers/pwm.h:38:0: style: The function 'pwm_set' is never used. [unusedFunction]
void pwm_set(TIM_TypeDef *TIM, uint8_t channel, uint8_t percentage){
^
/tmp/openpilot/panda/board/drivers/usb.h:474:0: style: The function 'usb_tick' is never used. [unusedFunction]
void usb_tick(void) {
^
/tmp/openpilot/panda/board/drivers/usb.h:659:0: style: The function 'usb_irqhandler' is never used. [unusedFunction]
void usb_irqhandler(void) {
^
/tmp/openpilot/panda/board/drivers/usb.h:928:0: style: The function 'can_tx_comms_resume_usb' is never used. [unusedFunction]
void can_tx_comms_resume_usb(void) {
^
/tmp/openpilot/panda/board/drivers/usb.h:937:0: style: The function 'usb_soft_disconnect' is never used. [unusedFunction]
void usb_soft_disconnect(bool enable) {
^
Error: Process completed with exit code 1.

@adeebshihadeh
Copy link
Contributor Author

I started misra-config in #1879, but I won't have time to finish it soon. It's up for grabs for the bounty if anyone wants to finish it.

@adeebshihadeh adeebshihadeh changed the title Extend MISRA coverage Finish MISRA coverage Feb 20, 2024
@adeebshihadeh adeebshihadeh moved this from Todo to In Progress in panda safety 1.0 Feb 21, 2024
@dzid26
Copy link
Contributor

dzid26 commented May 21, 2024

Seems like there is nothing left here anymore so I started playing with ppcheck 2.14 and fixing remaining violations that it produced.

Current status:

  • most rules have already open PRs and 12.2 has a Adeeb's draft PR, but
  • 8.7 is abandoned.
  • 2.5 doesn't compile doesn't pass CI
  • 1.2 has issues
  • unusedFunction should not be run on the multiple firmwares at once (pandas, jungle, etc)

I will be performing the integration and validation of the remaining rules so that they are easy to merge #1954.

This was referenced May 21, 2024
This was referenced May 30, 2024
@github-project-automation github-project-automation bot moved this from Open to Done in openpilot bounties Sep 13, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in panda safety 1.0 Sep 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Status: Done
Development

No branches or pull requests

3 participants