search

commaai / panda

code powering the comma.ai panda
MIT License
1.54k stars 780 forks source link

Finish MISRA coverage #1794

Closed adeebshihadeh closed 2 months ago

adeebshihadeh commented 10 months ago

In https://github.com/commaai/panda/pull/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.

Example for misra-c2012-2.2: f64ba24685fb3c71e37c720734d24d6b9d995617.

adeebshihadeh commented 9 months ago

@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 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 8 months ago

@0x41head I got the unused check running fast, but there are a few false positives (https://github.com/commaai/panda/pull/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 commented 8 months ago

I started misra-config in https://github.com/commaai/panda/pull/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.

dzid26 commented 5 months ago

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:

I will be performing the integration and validation of the remaining rules so that they are easy to merge https://github.com/commaai/panda/issues/1954.