mirror of https://github.com/ARMmbed/mbed-os.git
USBHost: Silence GCC warnings in USBHost.cpp
There are a few warnings thrown by GCC for this source file that I have attempted to correct. I would definitely appreciate feedback from others on these changes: * USBHost::usb_process() would attempt to write past the end of the deviceInited[] array in the following code snippet: if (i == MAX_DEVICE_CONNECTED) { USB_ERR("Too many device connected!!\r\n"); deviceInited[i] = false; The i variable is guaranteed to index 1 item past then end of this array since it only contains MAX_DEVICE_CONNECTED elements. I have removed the line which sets deviceInited[i] to false. Two questions result though: 1) What was the intent of this line of code and is it Ok that I just removed it or should it be replaced with something else? 2) I see no where else that elements in the deviceInited array are set to false except when all are set to false in the usbThread() method. Should there be code in DEVICE_DISCONNECTED_EVENT to do this as well? * USBHost::transferCompleted(volatile uint32_t addr) was comparing addr to NULL, which is of pointer type. GCC issues a warning on this since the types are different (void* being compared to uint32_t). I switched it to just compare with 0 instead. * There is a switch statement in USBHost::unqueueEndpoint() which is conditional on a ENDPOINT_TYPE enumeration but doesn't contain cases for all values in the enumeration. I just added a default case to simply break on other values to silence this GCC warning and allow the code to continue working as it did before. Is it Ok that this particular piece of code only handles these two particular cases? * USBHost::fillControlBuf() was generating a warning about possible alignment issues when accessing the setupPacket byte array as 16-bit half words instead. I changed the casting to silence the warnings and modified the declaration of the setupPacket field to make sure that it is at least 2-byte aligned for these 16-bit accesses.pull/67/head
parent
bfafd8a014
commit
ddb3fbe826
|
@ -96,7 +96,6 @@ void USBHost::usb_process() {
|
|||
|
||||
if (i == MAX_DEVICE_CONNECTED) {
|
||||
USB_ERR("Too many device connected!!\r\n");
|
||||
deviceInited[i] = false;
|
||||
usb_mutex.unlock();
|
||||
continue;
|
||||
}
|
||||
|
@ -287,7 +286,7 @@ void USBHost::transferCompleted(volatile uint32_t addr)
|
|||
{
|
||||
uint8_t state;
|
||||
|
||||
if(addr == NULL)
|
||||
if(addr == 0)
|
||||
return;
|
||||
|
||||
volatile HCTD* tdList = NULL;
|
||||
|
@ -482,6 +481,8 @@ void USBHost::unqueueEndpoint(USBEndpoint * ep)
|
|||
case INTERRUPT_ENDPOINT:
|
||||
tailInterruptEndpoint = prec;
|
||||
break;
|
||||
default:
|
||||
break;
|
||||
}
|
||||
}
|
||||
current->setState(USB_TYPE_FREE);
|
||||
|
@ -1158,7 +1159,8 @@ void USBHost::fillControlBuf(uint8_t requestType, uint8_t request, uint16_t valu
|
|||
setupPacket[0] = requestType;
|
||||
setupPacket[1] = request;
|
||||
//We are in LE so it's fine
|
||||
*((uint16_t*)&setupPacket[2]) = value;
|
||||
*((uint16_t*)&setupPacket[4]) = index;
|
||||
*((uint16_t*)&setupPacket[6]) = (uint32_t) len;
|
||||
uint16_t* setupPacketHalfWords = (uint16_t*)setupPacket;
|
||||
setupPacketHalfWords[1] = value;
|
||||
setupPacketHalfWords[2] = index;
|
||||
setupPacketHalfWords[3] = (uint32_t) len;
|
||||
}
|
||||
|
|
|
@ -252,7 +252,7 @@ private:
|
|||
#endif
|
||||
|
||||
// to store a setup packet
|
||||
uint8_t setupPacket[8];
|
||||
__attribute__((aligned(2))) uint8_t setupPacket[8];
|
||||
|
||||
typedef struct {
|
||||
uint8_t event_id;
|
||||
|
|
Loading…
Reference in New Issue