Misra 11.x: pointer usage. (commaai#250)

* RDLR, RDHR gone from safety

* back at not failing misra safety

* fix safety tests

* did not mean this

* Use get-bytes in pedal too

* Ignore Misra 11.5 and Fix 11.8 violaitons

* Fix 11.3 violations

* Neglect Misra 11.4, fix pointer issue in bootstub and rutn on Werror for pedal builds

* Fix Misra 11.6: can't assign number to void pointer

* Fix test after changing honda_moving name

* for loop is better

* Fix bugs from fbck

board/drivers/llcan.h

@@ -8,6 +8,13 @@
 // 5000 = 500 kbps
 #define can_speed_to_prescaler(x) (CAN_PCLK / CAN_QUANTA * 10U / (x))
 
+#define GET_BUS(msg) (((msg)->RDTR >> 4) & 0xFF)
+#define GET_LEN(msg) ((msg)->RDTR & 0xF)
+#define GET_ADDR(msg) ((((msg)->RIR & 4) != 0) ? ((msg)->RIR >> 3) : ((msg)->RIR >> 21))
+#define GET_BYTE(msg, b) (((int)(b) > 3) ? (((msg)->RDHR >> (8U * ((unsigned int)(b) % 4U))) & 0XFFU) : (((msg)->RDLR >> (8U * (unsigned int)(b))) & 0xFFU))
+#define GET_BYTES_04(msg) ((msg)->RDLR)
+#define GET_BYTES_48(msg) ((msg)->RDHR)
+
 void puts(const char *a);
 
 bool llcan_set_speed(CAN_TypeDef *CAN_obj, uint32_t speed, bool loopback, bool silent) {

board/drivers/usb.h

@@ -25,9 +25,9 @@ USB_Setup_TypeDef;
 
 void usb_init(void);
 int usb_cb_control_msg(USB_Setup_TypeDef *setup, uint8_t *resp, bool hardwired);
-int usb_cb_ep1_in(uint8_t *usbdata, int len, bool hardwired);
-void usb_cb_ep2_out(uint8_t *usbdata, int len, bool hardwired);
-void usb_cb_ep3_out(uint8_t *usbdata, int len, bool hardwired);
+int usb_cb_ep1_in(void *usbdata, int len, bool hardwired);
+void usb_cb_ep2_out(void *usbdata, int len, bool hardwired);
+void usb_cb_ep3_out(void *usbdata, int len, bool hardwired);
 void usb_cb_enumeration_complete(void);
 
 // **** supporting defines ****
@@ -404,7 +404,7 @@ void *USB_ReadPacket(void *dest, uint16_t len) {
   return ((void *)dest_copy);
 }
 
-void USB_WritePacket(const uint8_t *src, uint16_t len, uint32_t ep) {
+void USB_WritePacket(const void *src, uint16_t len, uint32_t ep) {
   #ifdef DEBUG_USB
   puts("writing ");
   hexdump(src, len);
@@ -420,10 +420,10 @@ void USB_WritePacket(const uint8_t *src, uint16_t len, uint32_t ep) {
   USBx_INEP(ep)->DIEPCTL |= (USB_OTG_DIEPCTL_CNAK | USB_OTG_DIEPCTL_EPENA);
 
   // load the FIFO
-  const uint8_t *src_copy = src;
+  const uint32_t *src_copy = (const uint32_t *)src;
   for (uint32_t i = 0; i < count32b; i++) {
-    USBx_DFIFO(ep) = *((__attribute__((__packed__)) uint32_t *)src_copy);
-    src_copy += 4;
+    USBx_DFIFO(ep) = *src_copy;
+    src_copy++;
   }
 }
 

board/main.c

@@ -140,7 +140,7 @@ int get_health_pkt(void *dat) {
   return sizeof(*health);
 }
 
-int usb_cb_ep1_in(uint8_t *usbdata, int len, bool hardwired) {
+int usb_cb_ep1_in(void *usbdata, int len, bool hardwired) {
   UNUSED(hardwired);
   CAN_FIFOMailBox_TypeDef *reply = (CAN_FIFOMailBox_TypeDef *)usbdata;
   int ilen = 0;
@@ -151,13 +151,14 @@ int usb_cb_ep1_in(uint8_t *usbdata, int len, bool hardwired) {
 }
 
 // send on serial, first byte to select the ring
-void usb_cb_ep2_out(uint8_t *usbdata, int len, bool hardwired) {
+void usb_cb_ep2_out(void *usbdata, int len, bool hardwired) {
   UNUSED(hardwired);
-  uart_ring *ur = get_ring_by_number(usbdata[0]);
+  uint8_t *usbdata8 = (uint8_t *)usbdata;
+  uart_ring *ur = get_ring_by_number(usbdata8[0]);
   if ((len != 0) && (ur != NULL)) {
-    if ((usbdata[0] < 2U) || safety_tx_lin_hook(usbdata[0] - 2U, usbdata + 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, usbdata[i])) {
+        while (!putc(ur, usbdata8[i])) {
           // wait
         }
       }
@@ -166,18 +167,16 @@ void usb_cb_ep2_out(uint8_t *usbdata, int len, bool hardwired) {
 }
 
 // send on CAN
-void usb_cb_ep3_out(uint8_t *usbdata, int len, bool hardwired) {
+void usb_cb_ep3_out(void *usbdata, int len, bool hardwired) {
   UNUSED(hardwired);
   int dpkt = 0;
-  for (dpkt = 0; dpkt < len; dpkt += 0x10) {
-    uint32_t *tf = (uint32_t*)(&usbdata[dpkt]);
-
-    // make a copy
+  uint32_t *d32 = (uint32_t *)usbdata;
+  for (dpkt = 0; dpkt < (len / 4); dpkt += 4) {
     CAN_FIFOMailBox_TypeDef to_push;
-    to_push.RDHR = tf[3];
-    to_push.RDLR = tf[2];
-    to_push.RDTR = tf[1];
-    to_push.RIR = tf[0];
+    to_push.RDHR = d32[dpkt + 3];
+    to_push.RDLR = d32[dpkt + 2];
+    to_push.RDTR = d32[dpkt + 1];
+    to_push.RIR = d32[dpkt];
 
     uint8_t bus_number = (to_push.RDTR >> 4) & CAN_BUS_NUM_MASK;
     can_send(&to_push, bus_number);
@@ -213,7 +212,7 @@ int usb_cb_control_msg(USB_Setup_TypeDef *setup, uint8_t *resp, bool hardwired)
     case 0xd0:
       // addresses are OTP
       if (setup->b.wValue.w == 1U) {
-        (void)memcpy(resp, (void *)0x1fff79c0, 0x10);
+        (void)memcpy(resp, (uint8_t *)0x1fff79c0, 0x10);
         resp_len = 0x10;
       } else {
         get_provision_chunk(resp);
@@ -492,6 +491,7 @@ int usb_cb_control_msg(USB_Setup_TypeDef *setup, uint8_t *resp, bool hardwired)
   return resp_len;
 }
 
+#ifndef EON
 int spi_cb_rx(uint8_t *data, int len, uint8_t *data_out) {
   // data[0]  = endpoint
   // data[2]  = length
@@ -521,7 +521,7 @@ int spi_cb_rx(uint8_t *data, int len, uint8_t *data_out) {
   }
   return resp_len;
 }
-
+#endif
 
 // ***************************** main code *****************************
 

board/pedal/Makefile

@@ -1,7 +1,7 @@
 # :set noet
 PROJ_NAME = comma
 
-CFLAGS  = -O2 -Wall -Wextra -Wstrict-prototypes -std=gnu11 -DPEDAL
+CFLAGS  = -O2 -Wall -Wextra -Wstrict-prototypes -Werror -std=gnu11 -DPEDAL
 CFLAGS += -mlittle-endian -mthumb -mcpu=cortex-m3
 CFLAGS += -msoft-float -DSTM32F2 -DSTM32F205xx
 CFLAGS += -I ../inc -I ../ -I ../../ -nostdlib -fno-builtin

board/pedal/main.c

@@ -155,11 +155,11 @@ void CAN1_RX0_IRQHandler(void) {
     int address = CAN->sFIFOMailBox[0].RIR >> 21;
     if (address == CAN_GAS_INPUT) {
       // softloader entry
-      if (CAN->sFIFOMailBox[0].RDLR == 0xdeadface) {
-        if (CAN->sFIFOMailBox[0].RDHR == 0x0ab00b1e) {
+      if (GET_BYTES_04(&CAN->sFIFOMailBox[0]) == 0xdeadface) {
+        if (GET_BYTES_48(&CAN->sFIFOMailBox[0]) == 0x0ab00b1e) {
           enter_bootloader_mode = ENTER_SOFTLOADER_MAGIC;
           NVIC_SystemReset();
-        } else if (CAN->sFIFOMailBox[0].RDHR == 0x02b00b1e) {
+        } else if (GET_BYTES_48(&CAN->sFIFOMailBox[0]) == 0x02b00b1e) {
           enter_bootloader_mode = ENTER_BOOTLOADER_MAGIC;
           NVIC_SystemReset();
         } else {
@@ -169,11 +169,8 @@ void CAN1_RX0_IRQHandler(void) {
 
       // normal packet
       uint8_t dat[8];
-      uint8_t *rdlr = (uint8_t *)&CAN->sFIFOMailBox[0].RDLR;
-      uint8_t *rdhr = (uint8_t *)&CAN->sFIFOMailBox[0].RDHR;
-      for (int i=0; i<4; i++) {
-        dat[i] = rdlr[i];
-        dat[i+4] = rdhr[i];
+      for (int i=0; i<8; i++) {
+        dat[i] = GET_BYTE(&CAN->sFIFOMailBox[0], i);
       }
       uint16_t value_0 = (dat[0] << 8) | dat[1];
       uint16_t value_1 = (dat[2] << 8) | dat[3];

board/provision.h

@@ -5,7 +5,7 @@
 // SHA1 checksum = 0x1C - 0x20
 
 void get_provision_chunk(uint8_t *resp) {
-  (void)memcpy(resp, (void *)0x1fff79e0, PROVISION_CHUNK_LEN);
+  (void)memcpy(resp, (uint8_t *)0x1fff79e0, PROVISION_CHUNK_LEN);
   if (memcmp(resp, "\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff", 0x20) == 0) {
     (void)memcpy(resp, "unprovisioned\x00\x00\x00testing123\x00\x00\xa3\xa6\x99\xec", 0x20);
   }

board/safety/safety_cadillac.h

@@ -10,13 +10,13 @@ const int CADILLAC_MAX_RATE_DOWN = 5;
 const int CADILLAC_DRIVER_TORQUE_ALLOWANCE = 50;
 const int CADILLAC_DRIVER_TORQUE_FACTOR = 4;
 
-int cadillac_ign = 0;
+bool cadillac_ign = 0;
 int cadillac_cruise_engaged_last = 0;
 int cadillac_rt_torque_last = 0;
 const int cadillac_torque_msgs_n = 4;
 int cadillac_desired_torque_last[CADILLAC_TORQUE_MSG_N] = {0};
 uint32_t cadillac_ts_last = 0;
-int cadillac_supercruise_on = 0;
+bool cadillac_supercruise_on = 0;
 struct sample_t cadillac_torque_driver;         // last few driver torques measured
 
 int cadillac_get_torque_idx(int addr, int array_size) {
@@ -28,20 +28,21 @@ static void cadillac_rx_hook(CAN_FIFOMailBox_TypeDef *to_push) {
   int addr = GET_ADDR(to_push);
 
   if (addr == 356) {
-    int torque_driver_new = ((to_push->RDLR & 0x7) << 8) | ((to_push->RDLR >> 8) & 0xFF);
+    int torque_driver_new = ((GET_BYTE(to_push, 0) & 0x7U) << 8) | (GET_BYTE(to_push, 1));
+
     torque_driver_new = to_signed(torque_driver_new, 11);
     // update array of samples
     update_sample(&cadillac_torque_driver, torque_driver_new);
   }
 
   // this message isn't all zeros when ignition is on
   if ((addr == 0x160) && (bus == 0)) {
-    cadillac_ign = to_push->RDLR > 0;
+    cadillac_ign = GET_BYTES_04(to_push) != 0;
   }
 
   // enter controls on rising edge of ACC, exit controls on ACC off
   if ((addr == 0x370) && (bus == 0)) {
-    int cruise_engaged = to_push->RDLR & 0x800000;  // bit 23
+    int cruise_engaged = GET_BYTE(to_push, 2) & 0x80;  // bit 23
     if (cruise_engaged && !cadillac_cruise_engaged_last) {
       controls_allowed = 1;
     }
@@ -53,7 +54,7 @@ static void cadillac_rx_hook(CAN_FIFOMailBox_TypeDef *to_push) {
 
   // know supercruise mode and block openpilot msgs if on
   if ((addr == 0x152) || (addr == 0x154)) {
-    cadillac_supercruise_on = (to_push->RDHR>>4) & 0x1;
+    cadillac_supercruise_on = (GET_BYTE(to_push, 4) & 0x10) != 0;
   }
 }
 
@@ -63,7 +64,7 @@ static int cadillac_tx_hook(CAN_FIFOMailBox_TypeDef *to_send) {
 
   // steer cmd checks
   if ((addr == 0x151) || (addr == 0x152) || (addr == 0x153) || (addr == 0x154)) {
-    int desired_torque = ((to_send->RDLR & 0x3f) << 8) + ((to_send->RDLR & 0xff00) >> 8);
+    int desired_torque = ((GET_BYTE(to_send, 0) & 0x3f) << 8) | GET_BYTE(to_send, 1);
     int violation = 0;
     uint32_t ts = TIM2->CNT;
     int idx = cadillac_get_torque_idx(addr, CADILLAC_TORQUE_MSG_N);

board/safety/safety_chrysler.h

@@ -18,16 +18,15 @@ static void chrysler_rx_hook(CAN_FIFOMailBox_TypeDef *to_push) {
 
   // Measured eps torque
   if (addr == 544) {
-    uint32_t rdhr = to_push->RDHR;
-    int torque_meas_new = ((rdhr & 0x7U) << 8) + ((rdhr & 0xFF00U) >> 8) - 1024U;
+    int torque_meas_new = ((GET_BYTE(to_push, 4) & 0x7U) << 8) + GET_BYTE(to_push, 5) - 1024U;
 
     // update array of samples
     update_sample(&chrysler_torque_meas, torque_meas_new);
   }
 
   // enter controls on rising edge of ACC, exit controls on ACC off
   if (addr == 0x1F4) {
-    int cruise_engaged = ((to_push->RDLR & 0x380000) >> 19) == 7;
+    int cruise_engaged = ((GET_BYTE(to_push, 2) & 0x38) >> 3) == 7;
     if (cruise_engaged && !chrysler_cruise_engaged_last) {
       controls_allowed = 1;
     }
@@ -57,8 +56,7 @@ static int chrysler_tx_hook(CAN_FIFOMailBox_TypeDef *to_send) {
 
   // LKA STEER
   if (addr == 0x292) {
-    uint32_t rdlr = to_send->RDLR;
-    int desired_torque = ((rdlr & 0x7U) << 8) + ((rdlr & 0xFF00U) >> 8) - 1024U;
+    int desired_torque = ((GET_BYTE(to_send, 0) & 0x7U) << 8) + GET_BYTE(to_send, 1) - 1024U;
     uint32_t ts = TIM2->CNT;
     bool violation = 0;
 

board/safety/safety_ford.h

@@ -9,22 +9,24 @@
 
 int ford_brake_prev = 0;
 int ford_gas_prev = 0;
-int ford_is_moving = 0;
+bool ford_moving = false;
 
 static void ford_rx_hook(CAN_FIFOMailBox_TypeDef *to_push) {
 
   int addr = GET_ADDR(to_push);
 
   if (addr == 0x217) {
     // wheel speeds are 14 bits every 16
-    ford_is_moving = 0xFCFF & (to_push->RDLR | (to_push->RDLR >> 16) |
-                               to_push->RDHR | (to_push->RDHR >> 16));
+    ford_moving = false;
+    for (int i = 0; i < 8; i += 2) {
+      ford_moving |= GET_BYTE(to_push, i) | (GET_BYTE(to_push, (int)(i + 1)) & 0xFCU);
+    }
   }
 
   // state machine to enter and exit controls
   if (addr == 0x83) {
-    bool cancel = (to_push->RDLR >> 8) & 0x1;
-    bool set_or_resume = (to_push->RDLR >> 28) & 0x3;
+    bool cancel = GET_BYTE(to_push, 1) & 0x1;
+    bool set_or_resume = GET_BYTE(to_push, 3) & 0x30;
     if (cancel) {
       controls_allowed = 0;
     }
@@ -36,16 +38,16 @@ static void ford_rx_hook(CAN_FIFOMailBox_TypeDef *to_push) {
   // exit controls on rising edge of brake press or on brake press when
   // speed > 0
   if (addr == 0x165) {
-    int brake = to_push->RDLR & 0x20;
-    if (brake && (!(ford_brake_prev) || ford_is_moving)) {
+    int brake = GET_BYTE(to_push, 0) & 0x20;
+    if (brake && (!(ford_brake_prev) || ford_moving)) {
       controls_allowed = 0;
     }
     ford_brake_prev = brake;
   }
 
   // exit controls on rising edge of gas press
   if (addr == 0x204) {
-    int gas = to_push->RDLR & 0xFF03;
+    int gas = (GET_BYTE(to_push, 0) & 0x03) | GET_BYTE(to_push, 1);
     if (gas && !(ford_gas_prev)) {
       controls_allowed = 0;
     }
@@ -64,15 +66,15 @@ static int ford_tx_hook(CAN_FIFOMailBox_TypeDef *to_send) {
   int tx = 1;
   // disallow actuator commands if gas or brake (with vehicle moving) are pressed
   // and the the latching controls_allowed flag is True
-  int pedal_pressed = ford_gas_prev || (ford_brake_prev && ford_is_moving);
+  int pedal_pressed = ford_gas_prev || (ford_brake_prev && ford_moving);
   bool current_controls_allowed = controls_allowed && !(pedal_pressed);
   int addr = GET_ADDR(to_send);
 
   // STEER: safety check
   if (addr == 0x3CA) {
     if (!current_controls_allowed) {
       // bits 7-4 need to be 0xF to disallow lkas commands
-      if (((to_send->RDLR >> 4) & 0xF) != 0xF) {
+      if ((GET_BYTE(to_send, 0) & 0xF0) != 0xF0) {
         tx = 0;
       }
     }
@@ -81,7 +83,7 @@ static int ford_tx_hook(CAN_FIFOMailBox_TypeDef *to_send) {
   // FORCE CANCEL: safety check only relevant when spamming the cancel button
   // ensuring that set and resume aren't sent
   if (addr == 0x83) {
-    if (((to_send->RDLR >> 28) & 0x3) != 0) {
+    if ((GET_BYTE(to_send, 3) & 0x30) != 0) {
       tx = 0;
     }
   }