-
-
Notifications
You must be signed in to change notification settings - Fork 7k
Ide 1.5.x warnings #1877
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ide 1.5.x warnings #1877
Conversation
This happened for AVR in 34885b0, this commit makes the SAM version identical again.
EthernetClass is a friend class of IPAddress, so it is allowed to use its _address attribute directly. However, it should be using IPAddress::raw_address() instead, like all the other friend classes do. This changes allows changing the _address attribute to fix some warnings next.
Previously, pointer casting was used, but this resulted in strict-aliasing warnings: IPAddress.h: In member function ‘IPAddress::operator uint32_t() const’: IPAddress.h:46:61: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing] operator uint32_t() const { return *((uint32_t*)_address); }; ^ IPAddress.h: In member function ‘bool IPAddress::operator==(const IPAddress&) const’: IPAddress.h:47:81: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing] bool operator==(const IPAddress& addr) const { return (*((uint32_t*)_address)) == (*((uint32_t*)addr._address)); }; ^ IPAddress.h:47:114: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing] bool operator==(const IPAddress& addr) const { return (*((uint32_t*)_address)) == (*((uint32_t*)addr._address)); }; Converting between unrelated types like this is commonly done using a union, which do not break the strict-aliasing rules. Using that union, inside IPAddress there is now an attribute _address.bytes for the raw byte arra, or _address.dword for the uint32_t version. Since we now have easy access to the uint32_t version, this also removes two memcpy invocations that can just become assignments. This patch does not change the generated code in any way, the compiler already optimized away the memcpy calls and the previous casts mean exactly the same. This is a different implementation of a part of arduino#1399 and it helps toward fixing arduino#1728.
In C++, true and false are language keywords, so there is no need to define them as macros. Including stdbool.h in C++ effectively changes nothing. In C, true, false and also the bool type are not available, but including stdbool.h will make them available. Using stdbool.h means that we get true, false and the bool type in whatever way the compiler thinks is best, which seems like a good idea to me. This also fixes the following compiler warnings if a .c file includes both stdbool.h and Arduino.h: warning: "true" redefined [enabled by default] #define true 0x1 warning: "false" redefined [enabled by default] #define false 0x0 This fixes arduino#1570 and helps toward fixing arduino#1728. This only changed the AVR core, the SAM core already doesn't define true and false (but doesn't include stdbool.h either).
All the while() loops that check for the SPI transfer to be complete have the semi-colon immediately after the closing parenthesis. This both causes a compiler warning of "warning: suggest a space before ';' or explicit braces around empty body in 'while' statement", and is considered a less-than-ideal programming practice. This patch breaks the semi-colon on to the next line, both eliminating the compiler error and making the code more readable. In all probability the test should be moved into a macro or a inlineable sub-routine.
Comment was duplicated from mkdir() and not updated.
peekNextDigit() returns an int, so it can return -1 in addition to all 256 possible bytes. By putting the result in a signe char, all bytes over 128 will be interpreted as "no bytes available". Furthermore, it seems that on SAM "char" is unsigned by default, causing the "if (c < 0)" line a bit further down to always be false. Using an int is more appropriate. A different fix for this issue was suggested in arduino#1399. This fix helps towards arduino#1728.
A bunch of functions have parameters they do not use, but which cannot be removed for API compatibility. In syscalls_sam3.c, there are a lot of these, so this adds an "UNUSED" macro which adds the "unused" variable attribute if supported (GCC specific), or is just a noop on other compilers. In CDC.cpp, there's only three of these variables, so this commit just forces a dummy evaluation of them to suppress the warnings. This helps towards arduino#1792.
len is an unsigned variable, so it will never be less than 0. This helps towards arduino#1792.
This makes the declaration of sprintf available, so the function is not implicitely declared, which triggers two compiler warnings. This helps towards arduino#1792
This helps towards arduino#1792
Members of this array are later passed to functions that accept non-const pointers. These functions probably don't modify their arguments, so a better solution would be to update those functions to accept const pointers. However, they look like third-party code, so that would require changing the code again on every update. Removing const here fixes at least the compiler warning for now. This helps towards arduino#1792.
The folks on the Italian forum pointed out that change the type of |
Any progress on this? |
Thanks for merging!
Ah, right, I hadn't looked at that part of the code indeed. I might try to fix those remaining warnings later, though I'm quite unfamiliar with the USB code and quite busy atm, so if anyone else wants to pick this up, that would be great. |
No problem, the remaining warnings may be not so trivial and needs more analysis to be solved properly. Here a recap if someone wants to take it over: |
This pullrequest fixes most warnings in the AVR and SAM cores. In particular, it:
-Wall -Wextra
warnings in the AVR core when compiling the blink sketch for the Uno using the bundled gcc 4.3.2, except for "only initialized variables can be placed into program memory area" (see Fix for "only initialized variables can be placed into program memory area" warning #1793).-Wall -Wextra
warnings in the SAM core when compiling the blink sketch for the Due using the bundled gcc 4.4.1, except for the USB-related warnings I reported in USB related warnings on SAM / Due #1874 and the one caused by Fix loops in the SAM banzai() reset function #1876.-Wall -Wextra
warnings in the AVR core when compiling the blink sketch for the Uno using an external gcc 4.8.2.This pullrequest also includes two commits for the SD library from #14 that were still applicable, which I slightly nudged to apply to some moved files, but did not otherwise modify.