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-2.3 & misra-c2012-2.4 #1801

Merged
merged 5 commits into from
Jan 16, 2024

Conversation

bongbui321
Copy link
Contributor

@bongbui321 bongbui321 commented Jan 15, 2024

For #1794, I included both the 2.3 and 2.4 since they are the same fix.

  1. Inline suppress the CANPacket_t type because it is used in other files and it is a false positives of cppcheck to check only if it is used in that specific file - board/can_definitions.h.
  2. Commented out USB_OTG_HostPortTypeDef type because they are commented out in board/drivers/usb.h and not used everywhere else

board/stm32fx/llusb.h Outdated Show resolved Hide resolved
@@ -15,7 +15,8 @@ const uint8_t PANDA_BUS_CNT = 4U;
#define CANPACKET_DATA_SIZE_MAX 8U
#endif

typedef struct {
// cppcheck-suppress [misra-c2012-2.3, misra-c2012-2.4]
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm, why do we have to do this here but not for health.h?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah it was a bit tricky to understand for me in the beginning too.

if you try to run my code, cppcheck is flagging misra-c2012 2.3 at the board/pedal/main.c file only not the other two test of board/main.c. I'm not entirely sure of the implementation of cppcheck, but I read through the code base and see that they would read through all of the include header files. For instance to check for misra_2.3 it would look through all of the typedef struct definition of the main file as well as the included files. In the board/pedal/main.c, if you follow the trail then it would include some header files that include the can_definitions.h file, while the pedal/main.c file doesn't use any header file that has health.h file in it.

I also did one experiment and put #include health.h in the board/pedal/main.c file and get this.

/home/openpilot/panda/board/health.h:35:1: style: misra violation (use --rule-texts=<file> to get proper output) [misra-c2012-2.3]
typedef struct __attribute__((packed)) {
^
/home/openpilot/panda/board/health.h:4:41: style: misra violation (use --rule-texts=<file> to get proper output) [misra-c2012-2.4]
struct __attribute__((packed)) health_t {

So I think it is a false positive of cppcheck.

Copy link
Contributor

Choose a reason for hiding this comment

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

it's not necessary, I just reverted it. please be more careful with changes like this in the future

@adeebshihadeh adeebshihadeh merged commit 7e8b829 into commaai:master Jan 16, 2024
9 checks passed
@bongbui321 bongbui321 deleted the misra_2.3_fix branch January 17, 2024 01:28
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.

2 participants