cancel
Showing results for 
Search instead for 
Did you mean: 

BUG report: USB Host HID class parser contains an error

funfunfun
Associate

I'm writing for stm32f417 MCU. The project is generated using STM32CubeMX with firmware package: STM32Cube FW_F4 V1.26.2 (the latest so far).

In the USB Host Library there is a file related to HID class:

<my_proj_path>\Middlewares\ST\STM32_USB_Host_Library\Class\HID\Src\usbh_hid_parser.c 

There is a function:

uint32_t HID_ReadItem(HID_Report_ItemTypedef *ri, uint8_t ndx)

It parses items of incoming reports. And there is a code in it:

/* read data bytes in little endian order */
  for (x = 0U; x < ((ri->size & 0x7U) ? (ri->size / 8U) + 1U : (ri->size / 8U)); x++)
  {
    val = (uint32_t)((uint32_t)(*data) << (x * 8U));
  }
  val = (val >> shift) & ((1U << ri->size) - 1U);

You can see here the multiple type casting. It's just meaningless overhead. But if you iterate through the loop assigning a value to the variable val it'll be overwritten on every iteration. Moreover the data pointer also stays unchanged during the loop.

At least this line should be changed to:

val |= (uint32_t)(*data++) << (x * 8U);

I also suggest you to test this function over different size and shift values (not only <= 8)

WBR

10 REPLIES 10

I think you meant "STM should not be taken too seriously."

STM device library support is frankly a joke, and needs a big disclaimer!

/** STM DEVICE LIBRARY Copyright 2015 **/
/* As we have a very short-term view, please don't take this code seriously! */

/* Have fun, love STM MCU Director, Paris *!