keyboardio / KeyboardioHID

A HID library for Arduino
MIT License
34 stars 17 forks source link

Remove all 'flexible arrays', which is not a valid C++ construct #3

Closed DonOregano closed 7 years ago

DonOregano commented 7 years ago

Flexible arrays is not a C++ feature. It appears that in GCC 4.x they are allowed, but when compiling this code with GCC 6.x a flexible array declaration will produce an error.

I have not delved deeply into this code, but it appears to me that the whole8, whole16 and whole32 members inside the unions are meant to provide an "alternate" view into the unions, that are just arrays. But since these are never used it should be safe to remove them.

I may be completely wrong about all this. If so, just ignore my pull request.

DonOregano commented 7 years ago

Another possibility is to replace all the [] with [0] which compiles as well. I believe this is a GCC extension to the C++ language, which basically allows for flexible arrays in C++ as well. Although, since there is no code that actually uses these arrays it is not possible for me to test that this works.

algernon commented 7 years ago

I'll do a test tonight, if all goes well.

algernon commented 7 years ago

A cursory look suggests that this should have no ill effect (but I'll try it on the prototype tonight, just in case), because the size of the struct should not change due to this change. If it does not work, an alternate approach would be to use uint8_t whole8[8]; uint16_t whole16[4]; uint32_t whole32[2]; - not as nice as the automatic sizes, but should compile down to the same thing.

But that won't be needed most likely.

algernon commented 7 years ago

Just tested the branch on the prototype, and everything seems to be working.

obra commented 7 years ago

Thanks! Sorry it took so long to merge