Skip to content

Commit ddb3fbe

Browse files
committed
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.
1 parent bfafd8a commit ddb3fbe

File tree

2 files changed

+8
-6
lines changed

2 files changed

+8
-6
lines changed

libraries/USBHost/USBHost/USBHost.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,6 @@ void USBHost::usb_process() {
9696

9797
if (i == MAX_DEVICE_CONNECTED) {
9898
USB_ERR("Too many device connected!!\r\n");
99-
deviceInited[i] = false;
10099
usb_mutex.unlock();
101100
continue;
102101
}
@@ -287,7 +286,7 @@ void USBHost::transferCompleted(volatile uint32_t addr)
287286
{
288287
uint8_t state;
289288

290-
if(addr == NULL)
289+
if(addr == 0)
291290
return;
292291

293292
volatile HCTD* tdList = NULL;
@@ -482,6 +481,8 @@ void USBHost::unqueueEndpoint(USBEndpoint * ep)
482481
case INTERRUPT_ENDPOINT:
483482
tailInterruptEndpoint = prec;
484483
break;
484+
default:
485+
break;
485486
}
486487
}
487488
current->setState(USB_TYPE_FREE);
@@ -1158,7 +1159,8 @@ void USBHost::fillControlBuf(uint8_t requestType, uint8_t request, uint16_t valu
11581159
setupPacket[0] = requestType;
11591160
setupPacket[1] = request;
11601161
//We are in LE so it's fine
1161-
*((uint16_t*)&setupPacket[2]) = value;
1162-
*((uint16_t*)&setupPacket[4]) = index;
1163-
*((uint16_t*)&setupPacket[6]) = (uint32_t) len;
1162+
uint16_t* setupPacketHalfWords = (uint16_t*)setupPacket;
1163+
setupPacketHalfWords[1] = value;
1164+
setupPacketHalfWords[2] = index;
1165+
setupPacketHalfWords[3] = (uint32_t) len;
11641166
}

libraries/USBHost/USBHost/USBHost.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@ class USBHost : public USBHALHost {
252252
#endif
253253

254254
// to store a setup packet
255-
uint8_t setupPacket[8];
255+
__attribute__((aligned(2))) uint8_t setupPacket[8];
256256

257257
typedef struct {
258258
uint8_t event_id;

0 commit comments

Comments
 (0)