Misra update (commaai#280)

* Updated cppcheck ref. New rules to be fixed * changed 2 MACRO names that had more than 31 characters in common * Fix newly popped 10.4: same type on arithmetic ops * Fix 18.4: operators should not be applied to an expression * ensure a NULL pointer isn't dereferenced
jyoung8607 · Sep 28, 2019 · fac0277 · fac0277
1 parent 5a04df6
commit fac0277
Show file tree

Hide file tree

Showing 8 changed files with 31 additions and 32 deletions.
diff --git a/board/config.h b/board/config.h
@@ -22,7 +22,7 @@
 
 #include <stdbool.h>
 #define NULL ((void*)0)
-#define COMPILE_TIME_ASSERT(pred) ((void)sizeof(char[1 - (2 * (!(pred)))]))
+#define COMPILE_TIME_ASSERT(pred) ((void)sizeof(char[1 - (2 * ((int)(!(pred))))]))
 
 #define MIN(a,b) \
  ({ __typeof__ (a) _a = (a); \

diff --git a/board/drivers/can.h b/board/drivers/can.h
@@ -298,7 +298,7 @@ void process_can(uint8_t can_number) {
           to_push.RDTR = (CAN->sTxMailBox[0].TDTR & 0xFFFF000FU) | ((CAN_BUS_RET_FLAG | bus_number) << 4);
           to_push.RDLR = CAN->sTxMailBox[0].TDLR;
           to_push.RDHR = CAN->sTxMailBox[0].TDHR;
-          can_send_errs += !can_push(&can_rx_q, &to_push);
+          can_send_errs += can_push(&can_rx_q, &to_push) ? 0U : 1U;
         }
 
         if ((CAN->TSR & CAN_TSR_TERR0) == CAN_TSR_TERR0) {
@@ -367,7 +367,7 @@ void can_rx(uint8_t can_number) {
     safety_rx_hook(&to_push);
 
     current_board->set_led(LED_BLUE, true);
-    can_send_errs += !can_push(&can_rx_q, &to_push);
+    can_send_errs += can_push(&can_rx_q, &to_push) ? 0U : 1U;
 
     // next
     CAN->RF0R |= CAN_RF0R_RFOM0;
@@ -393,10 +393,9 @@ void can_send(CAN_FIFOMailBox_TypeDef *to_push, uint8_t bus_number) {
       // bus number isn't passed through
       to_push->RDTR &= 0xF;
       if ((bus_number == 3U) && (can_num_lookup[3] == 0xFFU)) {
-        // TODO: why uint8 bro? only int8?
-        gmlan_send_errs += !bitbang_gmlan(to_push);
+        gmlan_send_errs += bitbang_gmlan(to_push) ? 0U : 1U;
       } else {
-        can_fwd_errs += !can_push(can_queues[bus_number], to_push);
+        can_fwd_errs += can_push(can_queues[bus_number], to_push) ? 0U : 1U;
         process_can(CAN_NUM_FROM_BUS_NUM(bus_number));
       }
     }

diff --git a/board/drivers/uart.h b/board/drivers/uart.h
@@ -327,7 +327,7 @@ void putui(uint32_t i) {
     idx--;
     i_copy /= 10;
   } while (i_copy != 0U);
-  puts(str + idx + 1U);
+  puts(&str[idx + 1U]);
 }
 
 void puth(unsigned int i) {
@@ -345,10 +345,12 @@ void puth2(unsigned int i) {
 }
 
 void hexdump(const void *a, int l) {
-  for (int i=0; i < l; i++) {
-    if ((i != 0) && ((i & 0xf) == 0)) puts("\n");
-    puth2(((const unsigned char*)a)[i]);
-    puts(" ");
+  if (a != NULL) {
+    for (int i=0; i < l; i++) {
+      if ((i != 0) && ((i & 0xf) == 0)) puts("\n");
+      puth2(((const unsigned char*)a)[i]);
+      puts(" ");
+    }
   }
   puts("\n");
 }
diff --git a/board/drivers/usb.h b/board/drivers/usb.h
@@ -439,7 +439,7 @@ void USB_WritePacket_EP0(uint8_t *src, uint16_t len) {
   USB_WritePacket(src, wplen, 0);
 
   if (wplen < len) {
-    ep0_txdata = src + wplen;
+    ep0_txdata = &src[wplen];
     ep0_txlen = len - wplen;
     USBx_DEVICE->DIEPEMPMSK |= 1;
   } else {
@@ -985,7 +985,7 @@ void usb_irqhandler(void) {
       if ((ep0_txlen != 0U) && ((USBx_INEP(0)->DTXFSTS & USB_OTG_DTXFSTS_INEPTFSAV) >= 0x40U)) {
         uint16_t len = MIN(ep0_txlen, 0x40);
         USB_WritePacket(ep0_txdata, len, 0);
-        ep0_txdata += len;
+        ep0_txdata = &ep0_txdata[len];
         ep0_txlen -= len;
         if (ep0_txlen == 0U) {
           ep0_txdata = NULL;

diff --git a/board/main.c b/board/main.c
@@ -1,4 +1,4 @@
-//#define EON 
+//#define EON
 //#define PANDA
 
 // ********************* Includes *********************
@@ -134,7 +134,7 @@ void set_safety_mode(uint16_t mode, int16_t param) {
           }
           can_silent = ALL_CAN_LIVE;
           break;
-      }          
+      }
     if (safety_ignition_hook() != -1) {
       // if the ignition hook depends on something other than the started GPIO
       // we have to disable power savings (fix for GM and Tesla)
@@ -195,7 +195,7 @@ int get_health_pkt(void *dat) {
   health->can_fwd_errs_pkt = can_fwd_errs;
   health->gmlan_send_errs_pkt = gmlan_send_errs;
   health->car_harness_status_pkt = car_harness_status;
-  
+
   return sizeof(*health);
 }
 
@@ -215,7 +215,7 @@ void usb_cb_ep2_out(void *usbdata, int len, bool hardwired) {
   uint8_t *usbdata8 = (uint8_t *)usbdata;
   uart_ring *ur = get_ring_by_number(usbdata8[0]);
   if ((len != 0) && (ur != NULL)) {
-    if ((usbdata8[0] < 2U) || safety_tx_lin_hook(usbdata8[0] - 2U, usbdata8 + 1, len - 1)) {
+    if ((usbdata8[0] < 2U) || safety_tx_lin_hook(usbdata8[0] - 2U, &usbdata8[1], len - 1)) {
       for (int i = 1; i < len; i++) {
         while (!putc(ur, usbdata8[i])) {
           // wait
@@ -346,7 +346,7 @@ int usb_cb_control_msg(USB_Setup_TypeDef *setup, uint8_t *resp, bool hardwired)
         } else {
           // Disable OBD CAN
           current_board->set_can_mode(CAN_MODE_NORMAL);
-        }        
+        }
       } else {
         if (setup->b.wValue.w == 1U) {
           // GMLAN ON
@@ -362,7 +362,7 @@ int usb_cb_control_msg(USB_Setup_TypeDef *setup, uint8_t *resp, bool hardwired)
         }
       }
       break;
-      
+
     // **** 0xdc: set safety mode
     case 0xdc:
       // Blocked over WiFi.
@@ -591,8 +591,8 @@ void __attribute__ ((noinline)) enable_fpu(void) {
 uint64_t tcnt = 0;
 
 // go into NOOUTPUT when the EON does not send a heartbeat for this amount of seconds.
-#define EON_HEARTBEAT_THRESHOLD_IGNITION_ON 5U
-#define EON_HEARTBEAT_THRESHOLD_IGNITION_OFF 2U
+#define EON_HEARTBEAT_IGNITION_CNT_ON 5U
+#define EON_HEARTBEAT_IGNITION_CNT_OFF 2U
 
 // called once per second
 // cppcheck-suppress unusedFunction ; used in headers not included in cppcheck
@@ -629,7 +629,7 @@ void TIM3_IRQHandler(void) {
 
     // check heartbeat counter if we are running EON code. If the heartbeat has been gone for a while, go to NOOUTPUT safety mode.
     #ifdef EON
-    if (heartbeat_counter >= (current_board->check_ignition() ? EON_HEARTBEAT_THRESHOLD_IGNITION_ON : EON_HEARTBEAT_THRESHOLD_IGNITION_OFF)) {
+    if (heartbeat_counter >= (current_board->check_ignition() ? EON_HEARTBEAT_IGNITION_CNT_ON : EON_HEARTBEAT_IGNITION_CNT_OFF)) {
       puts("EON hasn't sent a heartbeat for 0x"); puth(heartbeat_counter); puts(" seconds. Safety is set to NOOUTPUT mode.\n");
       set_safety_mode(SAFETY_NOOUTPUT, 0U);
     }
@@ -651,7 +651,7 @@ int main(void) {
   detect_configuration();
   detect_board_type();
   adc_init();
-  
+
   // print hello
   puts("\n\n\n************************ MAIN START ************************\n");
 

diff --git a/tests/misra/coverage_table b/tests/misra/coverage_table
@@ -92,7 +92,7 @@
 16.6    X (Addon)
 16.7    X (Addon)
 17.1    X (Addon)
-17.2    
+17.2    X (Addon)
 17.3    
 17.4    
 17.5    X (Cppcheck)
@@ -102,7 +102,7 @@
 18.1    X (Cppcheck)
 18.2    X (Cppcheck)
 18.3    X (Cppcheck)
-18.4    
+18.4    X (Addon)
 18.5    X (Addon)
 18.6    X (Cppcheck)
 18.7    X (Addon)

diff --git a/tests/misra/suppressions.txt b/tests/misra/suppressions.txt
@@ -1,7 +1,5 @@
 # Advisory: union types can be used
 misra.19.2
-# FIXME: add it back when fixed in cppcheck. Macro identifiers are unique but it false triggers on defines in #ifdef..#else conditions
-misra.5.4
 # Advisory: casting from void pointer to type pointer is ok. Done by STM libraries as well
 misra.11.4
 # Advisory: casting from void pointer to type pointer is ok. Done by STM libraries as well

diff --git a/tests/misra/test_misra.sh b/tests/misra/test_misra.sh
@@ -4,7 +4,7 @@ mkdir /tmp/misra || true
 git clone https://github.com/danmar/cppcheck.git || true
 cd cppcheck
 git fetch
-git checkout 862c4ef87b109ae86c2d5f12769b7c8d199f35c5
+git checkout ff7dba91e177dfb712477faddb9e91bece7e743c
 make -j4
 cd ../../../
 
@@ -20,8 +20,8 @@ tests/misra/cppcheck/cppcheck -DPANDA -UPEDAL -DCAN3 -DUID_BASE -DEON \
 python tests/misra/cppcheck/addons/misra.py board/main.c.dump 2> /tmp/misra/misra_output.txt || true
 
 # strip (information) lines
-cppcheck_output=$( cat /tmp/misra/cppcheck_output.txt | grep -v "(information) " ) || true
-misra_output=$( cat /tmp/misra/misra_output.txt | grep -v "(information) " ) || true
+cppcheck_output=$( cat /tmp/misra/cppcheck_output.txt | grep -v ": information: " ) || true
+misra_output=$( cat /tmp/misra/misra_output.txt | grep -v ": information: " ) || true
 
 
 printf "\nPEDAL CODE\n"
@@ -33,8 +33,8 @@ tests/misra/cppcheck/cppcheck -UPANDA -DPEDAL -UCAN3 \
 python tests/misra/cppcheck/addons/misra.py board/pedal/main.c.dump 2> /tmp/misra/misra_pedal_output.txt || true
 
 # strip (information) lines
-cppcheck_pedal_output=$( cat /tmp/misra/cppcheck_pedal_output.txt | grep -v "(information) " ) || true
-misra_pedal_output=$( cat /tmp/misra/misra_pedal_output.txt | grep -v "(information) " ) || true
+cppcheck_pedal_output=$( cat /tmp/misra/cppcheck_pedal_output.txt | grep -v ": information: " ) || true
+misra_pedal_output=$( cat /tmp/misra/misra_pedal_output.txt | grep -v ": information: " ) || true
 
 if [[ -n "$misra_output" ]] || [[ -n "$cppcheck_output" ]]
 then