Add -Werror to acceptance builds for C and CPP (#4369)

Use platform.local.txt to add -Werror to GCC for the build of all
code.  Any warnings on a submitted patch will cause an error.

Several examples and libraries had warnings/errors (missing returns
on functions, types, etc.).  Clean those up with this commit as well.

libraries/ArduinoOTA/examples/OTALeds/OTALeds.ino

@@ -55,7 +55,7 @@ void setup() {
                           }
                         });
 
-   ArduinoOTA.onError([](ota_error_t error) { ESP.restart(); });
+   ArduinoOTA.onError([](ota_error_t error) { (void)error; ESP.restart(); });
 
    /* setup the OTA server */
    ArduinoOTA.begin();

libraries/DNSServer/examples/CaptivePortalAdvanced/CaptivePortalAdvanced.ino

@@ -44,10 +44,10 @@ IPAddress netMsk(255, 255, 255, 0);
 boolean connect;
 
 /** Last time I tried to connect to WLAN */
-long lastConnectTry = 0;
+unsigned long lastConnectTry = 0;
 
 /** Current WLAN status */
-int status = WL_IDLE_STATUS;
+unsigned int status = WL_IDLE_STATUS;
 
 void setup() {
   delay(1000);
@@ -95,7 +95,7 @@ void loop() {
     lastConnectTry = millis();
   }
   {
-    int s = WiFi.status();
+    unsigned int s = WiFi.status();
     if (s == 0 && millis() > (lastConnectTry + 60000) ) {
       /* If WLAN disconnected and idle try to connect */
       /* Don't set retry time too low as retry interfere the softAP operation */

libraries/DNSServer/examples/CaptivePortalAdvanced/tools.ino

@@ -1,6 +1,6 @@
 /** Is this an IP? */
 boolean isIp(String str) {
-  for (int i = 0; i < str.length(); i++) {
+  for (size_t i = 0; i < str.length(); i++) {
     int c = str.charAt(i);
     if (c != '.' && (c < '0' || c > '9')) {
       return false;

libraries/ESP8266AVRISP/src/ESP8266AVRISP.cpp

@@ -39,8 +39,8 @@ extern "C" {
 #define beget16(addr) (*addr * 256 + *(addr+1))
 
 ESP8266AVRISP::ESP8266AVRISP(uint16_t port, uint8_t reset_pin, uint32_t spi_freq, bool reset_state, bool reset_activehigh):
-    _reset_pin(reset_pin), _reset_state(reset_state), _spi_freq(spi_freq), _reset_activehigh(reset_activehigh),
+    _spi_freq(spi_freq), _server(WiFiServer(port)), _state(AVRISP_STATE_IDLE),
-    _server(WiFiServer(port)), _state(AVRISP_STATE_IDLE)
+    _reset_pin(reset_pin), _reset_state(reset_state), _reset_activehigh(reset_activehigh)
 {
     pinMode(_reset_pin, OUTPUT);
     setReset(_reset_state);
@@ -71,6 +71,7 @@ AVRISPState_t ESP8266AVRISP::update() {
                 ip_addr_t lip;
                 lip.addr = _client.remoteIP();
                 AVRISP_DEBUG("client connect %d.%d.%d.%d:%d", IP2STR(&lip), _client.remotePort());
+                (void) lip; // Avoid unused warning when not in debug mode
                 _client.setTimeout(100); // for getch()
                 _state = AVRISP_STATE_PENDING;
                 _reject_incoming();
@@ -136,10 +137,9 @@ void ESP8266AVRISP::fill(int n) {
 }
 
 uint8_t ESP8266AVRISP::spi_transaction(uint8_t a, uint8_t b, uint8_t c, uint8_t d) {
-    uint8_t n;
     SPI.transfer(a);
-    n = SPI.transfer(b);
+    SPI.transfer(b);
-    n = SPI.transfer(c);
+    SPI.transfer(c);
     return SPI.transfer(d);
 }
 
@@ -233,7 +233,6 @@ void ESP8266AVRISP::end_pmode() {
 }
 
 void ESP8266AVRISP::universal() {
-    int w;
     uint8_t ch;
 
     fill(4);
@@ -265,8 +264,6 @@ int ESP8266AVRISP::addr_page(int addr) {
 
 
 void ESP8266AVRISP::write_flash(int length) {
-    uint32_t started = millis();
-
     fill(length);
 
     if (Sync_CRC_EOP == getch()) {
@@ -331,7 +328,6 @@ void ESP8266AVRISP::program_page() {
     int length = 256 * getch();
     length += getch();
     char memtype = getch();
-    char buf[100];
     // flash memory @here, (length) bytes
     if (memtype == 'F') {
         write_flash(length);
@@ -390,7 +386,6 @@ void ESP8266AVRISP::eeprom_read_page(int length) {
 }
 
 void ESP8266AVRISP::read_page() {
-    char result = (char)Resp_STK_FAILED;
     int length = 256 * getch();
     length += getch();
     char memtype = getch();
@@ -424,9 +419,13 @@ void ESP8266AVRISP::read_signature() {
 
 // It seems ArduinoISP is based on the original STK500 (not v2)
 // but implements only a subset of the commands.
-int ESP8266AVRISP::avrisp() {
+void ESP8266AVRISP::avrisp() {
     uint8_t data, low, high;
     uint8_t ch = getch();
+    // Avoid set but not used warning.  Leaving them in as it helps document the code
+    (void) data;
+    (void) low;
+    (void) high;
     // AVRISP_DEBUG("CMD 0x%02x", ch);
     switch (ch) {
     case Cmnd_STK_GET_SYNC:
@@ -517,7 +516,7 @@ int ESP8266AVRISP::avrisp() {
 
       // anything else we will return STK_UNKNOWN
     default:
-        AVRISP_DEBUG("??!?");
+        AVRISP_DEBUG("?!?");
         error++;
         if (Sync_CRC_EOP == getch()) {
             _client.print((char)Resp_STK_UNKNOWN);

libraries/ESP8266AVRISP/src/ESP8266AVRISP.h

@@ -71,7 +71,7 @@ protected:
 
     inline void _reject_incoming(void);     // reject any incoming tcp connections
 
-    int avrisp(void);           // handle incoming STK500 commands
+    void avrisp(void);           // handle incoming STK500 commands
 
     uint8_t getch(void);        // retrieve a character from the remote end
     uint8_t spi_transaction(uint8_t, uint8_t, uint8_t, uint8_t);

libraries/ESP8266LLMNR/ESP8266LLMNR.cpp

@@ -87,10 +87,12 @@ bool LLMNRResponder::begin(const char* hostname) {
     _hostname.toLowerCase();
 
     _sta_got_ip_handler = WiFi.onStationModeGotIP([this](const WiFiEventStationModeGotIP& event){
+        (void) event;
         _restart();
     });
 
     _sta_disconnected_handler = WiFi.onStationModeDisconnected([this](const WiFiEventStationModeDisconnected& event) {
+        (void) event;
         _restart();
     });
 
@@ -122,6 +124,7 @@ bool LLMNRResponder::_restart() {
     _conn->setMulticastTTL(LLMNR_MULTICAST_TTL);
     _conn->onRx(std::bind(&LLMNRResponder::_process_packet, this));
     _conn->connect(multicast_addr, LLMNR_PORT);
+    return true;
 }
 
 void LLMNRResponder::_process_packet() {
@@ -242,8 +245,8 @@ void LLMNRResponder::_process_packet() {
 
     // Header
     uint8_t header[] = {
-        id >> 8, id & 0xff, // ID
+        (uint8_t)(id >> 8), (uint8_t)(id & 0xff), // ID
-        FLAGS_QR >> 8, 0, // FLAGS
+        (uint8_t)(FLAGS_QR >> 8), 0, // FLAGS
         0, 1, // QDCOUNT
         0, !!have_rr, // ANCOUNT
         0, 0, // NSCOUNT
@@ -269,7 +272,7 @@ void LLMNRResponder::_process_packet() {
             0, 1, // CLASS (IN)
             0, 0, 0, 30, // TTL (30 seconds)
             0, 4, // RDLENGTH
-            ip & 0xff, (ip >> 8) & 0xff, (ip >> 16) & 0xff, (ip >> 24) & 0xff, // RDATA
+            (uint8_t)(ip & 0xff), (uint8_t)((ip >> 8) & 0xff), (uint8_t)((ip >> 16) & 0xff), (uint8_t)((ip >> 24) & 0xff) // RDATA
         };
         _conn->append(reinterpret_cast<const char*>(rr), sizeof(rr));
     }

libraries/ESP8266WebServer/examples/FSBrowser/FSBrowser.ino

@@ -77,7 +77,7 @@ bool handleFileRead(String path){
     if(SPIFFS.exists(pathWithGz))
       path += ".gz";
     File file = SPIFFS.open(path, "r");
-    size_t sent = server.streamFile(file, contentType);
+    server.streamFile(file, contentType);
     file.close();
     return true;
   }

libraries/ESP8266WiFi/examples/NTPClient/NTPClient.ino

@@ -129,7 +129,7 @@ void loop()
 }
 
 // send an NTP request to the time server at the given address
-unsigned long sendNTPpacket(IPAddress& address)
+void sendNTPpacket(IPAddress& address)
 {
   Serial.println("sending NTP packet...");
   // set all bytes in the buffer to 0

libraries/Ethernet/examples/BarometricPressureWebServer/BarometricPressureWebServer.ino

@@ -204,7 +204,7 @@ unsigned int readRegister(byte registerName, int numBytes) {
   // take the chip select low to select the device:
   digitalWrite(chipSelectPin, LOW);
   // send the device the register you want to read:
-  int command = SPI.transfer(registerName);
+  SPI.transfer(registerName);
   // send a value of 0 to read the first byte returned:
   inByte = SPI.transfer(0x00);
 

libraries/Ethernet/examples/UdpNtpClient/UdpNtpClient.ino

@@ -108,7 +108,7 @@ void loop()
 }
 
 // send an NTP request to the time server at the given address
-unsigned long sendNTPpacket(char* address)
+void sendNTPpacket(char* address)
 {
   // set all bytes in the buffer to 0
   memset(packetBuffer, 0, NTP_PACKET_SIZE);

libraries/Ethernet/src/Dns.cpp

@@ -282,9 +282,12 @@ uint16_t DNSClient::ProcessResponse(uint16_t aTimeout, IPAddress& aAddress)
     }
     iUdp.read(header, DNS_HEADER_SIZE);
 
-    uint16_t header_flags = htons(*((uint16_t*)&header[2]));
+    uint16_t staging; // Staging used to avoid type-punning warnings
+    memcpy(&staging, &header[2], sizeof(uint16_t));
+    uint16_t header_flags = htons(staging);
+    memcpy(&staging, &header[0], sizeof(uint16_t));
     // Check that it's a response to this request
-    if ( ( iRequestId != (*((uint16_t*)&header[0])) ) ||
+    if ( ( iRequestId != staging ) ||
         ((header_flags & QUERY_RESPONSE_MASK) != (uint16_t)RESPONSE_FLAG) )
     {
         // Mark the entire packet as read
@@ -301,7 +304,8 @@ uint16_t DNSClient::ProcessResponse(uint16_t aTimeout, IPAddress& aAddress)
     }
 
     // And make sure we've got (at least) one answer
-    uint16_t answerCount = htons(*((uint16_t*)&header[6]));
+    memcpy(&staging, &header[6], sizeof(uint16_t));
+    uint16_t answerCount = htons(staging);
     if (answerCount == 0 )
     {
         // Mark the entire packet as read
@@ -310,7 +314,8 @@ uint16_t DNSClient::ProcessResponse(uint16_t aTimeout, IPAddress& aAddress)
     }
 
     // Skip over any questions
-    for (uint16_t i =0; i < htons(*((uint16_t*)&header[4])); i++)
+    memcpy(&staging, &header[4], sizeof(uint16_t));
+    for (uint16_t i =0; i < htons(staging); i++)
     {
         // Skip over the name
         uint8_t len;

libraries/Ethernet/src/utility/socket.cpp

@@ -364,6 +364,7 @@ uint16_t recvfrom(SOCKET s, uint8_t *buf, uint16_t len, uint8_t *addr, uint16_t
  */
 void flush(SOCKET s) {
   // TODO
+  (void) s;
 }
 
 uint16_t igmpsend(SOCKET s, const uint8_t * buf, uint16_t len)

libraries/SD/src/SD.cpp

@@ -241,6 +241,8 @@ boolean callback_pathExists(SdFile& parentDir, char *filePathComponent,
 
   */
   SdFile child;
+  (void) isLastComponent;
+  (void) object;
 
   boolean exists = child.open(parentDir, filePathComponent, O_RDONLY);
   
@@ -310,6 +312,8 @@ boolean callback_openPath(SdFile& parentDir, char *filePathComponent,
 
 boolean callback_remove(SdFile& parentDir, char *filePathComponent, 
 			boolean isLastComponent, void *object) {
+  (void) object;
+
   if (isLastComponent) {
     return SdFile::remove(parentDir, filePathComponent);
   }
@@ -318,6 +322,7 @@ boolean callback_remove(SdFile& parentDir, char *filePathComponent,
 
 boolean callback_rmdir(SdFile& parentDir, char *filePathComponent, 
 			boolean isLastComponent, void *object) {
+  (void) object;
   if (isLastComponent) {
     SdFile f;
     if (!f.open(parentDir, filePathComponent, O_READ)) return false;

libraries/SD/src/utility/Sd2Card.cpp

@@ -393,7 +393,6 @@ uint8_t Sd2Card::readBlock(uint32_t block, uint8_t* dst) {
  */
 uint8_t Sd2Card::readData(uint32_t block,
         uint16_t offset, uint16_t count, uint8_t* dst) {
-  uint16_t n;
   if (count == 0) return true;
   if ((count + offset) > 512) {
     goto fail;
@@ -414,6 +413,8 @@ uint8_t Sd2Card::readData(uint32_t block,
   }
 
 #ifdef OPTIMIZE_HARDWARE_SPI
+  uint16_t n;
+
   // start first spi transfer
   SPDR = 0XFF;
 

libraries/SD/src/utility/SdFile.cpp

@@ -259,11 +259,12 @@ uint8_t SdFile::make83Name(const char* str, uint8_t* name) {
       i = 8;   // place for extension
     } else {
       // illegal FAT characters
-      uint8_t b;
 #if defined(__AVR__)
+      uint8_t b;
       PGM_P p = PSTR("|<>^+=?/[];,*\"\\");
       while ((b = pgm_read_byte(p++))) if (b == c) return false;
 #elif defined(__arm__)
+      uint8_t b;
       const uint8_t valid[] = "|<>^+=?/[];,*\"\\";
       const uint8_t *p = valid;
       while ((b = *p++)) if (b == c) return false;
@@ -905,7 +906,7 @@ uint8_t SdFile::rmRfStar(void) {
       if (!f.remove()) return false;
     }
     // position to next entry if required
-    if (curPosition_ != (32*(index + 1))) {
+    if (curPosition_ != (32*((uint32_t)index + 1))) {
       if (!seekSet(32*(index + 1))) return false;
     }
   }

libraries/SPISlave/examples/SPISlave_Test/SPISlave_Test.ino

@@ -27,11 +27,12 @@ void setup()
     // It's up to the user to implement protocol for handling data length
     SPISlave.onData([](uint8_t * data, size_t len) {
         String message = String((char *)data);
+        (void) len;
         if(message.equals("Hello Slave!")) {
             SPISlave.setData("Hello Master!");
         } else if(message.equals("Are you alive?")) {
             char answer[33];
-            sprintf(answer,"Alive for %u seconds!", millis() / 1000);
+            sprintf(answer,"Alive for %lu seconds!", millis() / 1000);
             SPISlave.setData(answer);
         } else {
             SPISlave.setData("Say what?");

libraries/SPISlave/src/hspi_slave.c

@@ -55,7 +55,6 @@ void ICACHE_RAM_ATTR _hspi_slave_isr_handler(void *arg)
         if((status & SPISWBIS) != 0 && (_hspi_slave_rx_data_cb)) {
             uint8_t i;
             uint32_t data;
-            uint8_t buffer[33];
             _hspi_slave_buffer[32] = 0;
             for(i=0; i<8; i++) {
                 data=SPI1W(i);

libraries/Servo/src/esp8266/Servo.cpp

@@ -251,8 +251,6 @@ uint8_t Servo::attach(int pin, uint16_t minUs, uint16_t maxUs)
 
 void Servo::detach()
 {
-    ServoTimerSequence timerId;
-
     if (s_servos[_servoIndex].info.isActive) {
         s_servos[_servoIndex].info.isDetaching = true;
     }

libraries/TFT_Touch_Shield_V2/TFTv2.cpp

@@ -34,10 +34,10 @@ void TFT::TFTinit (void)
     
     TFT_CS_HIGH;
     TFT_DC_HIGH;
-    INT8U i=0, TFTDriver=0;
+    INT8U i=0;
     for(i=0;i<3;i++)
     {
-        TFTDriver = readID();
+        readID();
     }
     delay(500);
     sendCMD(0x01);
@@ -204,10 +204,10 @@ void TFT::fillScreen(INT16U XL, INT16U XR, INT16U YU, INT16U YD, INT16U color)
         YD = YU^YD;
         YU = YU^YD;
     }
-    XL = constrain(XL, MIN_X,MAX_X);
+    XL = constrain((int)XL, (int)MIN_X, (int)MAX_X);
-    XR = constrain(XR, MIN_X,MAX_X);
+    XR = constrain((int)XR, (int)MIN_X, (int)MAX_X);
-    YU = constrain(YU, MIN_Y,MAX_Y);
+    YU = constrain((int)YU, (int)MIN_Y, (int)MAX_Y);
-    YD = constrain(YD, MIN_Y,MAX_Y);
+    YD = constrain((int)YD, (int)MIN_Y, (int)MAX_Y);
 
     XY = (XR-XL+1);
     XY = XY*(YD-YU+1);
@@ -295,12 +295,12 @@ void TFT::drawChar( INT8U ascii, INT16U poX, INT16U poY,INT16U size, INT16U fgco
     }
 }
 
-void TFT::drawString(char *string,INT16U poX, INT16U poY, INT16U size,INT16U fgcolor)
+void TFT::drawString(const char *string,INT16U poX, INT16U poY, INT16U size,INT16U fgcolor)
 {
     while(*string)
     {
         drawChar(*string, poX, poY, size, fgcolor);
-        *string++;
+        string++;
 
         if(poX < MAX_X)
         {
@@ -321,7 +321,7 @@ INT16U length,INT16U color)
     setCol(poX,poX + length);
     setPage(poY,poY);
     sendCMD(0x2c);
-    for(int i=0; i<length; i++)
+    for(INT16U i=0; i<length; i++)
     sendData(color);
 }
 
@@ -353,7 +353,7 @@ void TFT::drawVerticalLine( INT16U poX, INT16U poY, INT16U length,INT16U color)
     setCol(poX,poX);
     setPage(poY,poY+length);
     sendCMD(0x2c);
-    for(int i=0; i<length; i++)
+    for(INT16U i=0; i<length; i++)
     sendData(color);
 }
 

libraries/TFT_Touch_Shield_V2/TFTv2.h

@@ -194,7 +194,7 @@ public:
 	INT8U readID(void);
 	
 	void drawChar(INT8U ascii,INT16U poX, INT16U poY,INT16U size, INT16U fgcolor);
-    void drawString(char *string,INT16U poX, INT16U poY,INT16U size,INT16U fgcolor);
+    void drawString(const char *string,INT16U poX, INT16U poY,INT16U size,INT16U fgcolor);
 	void fillRectangle(INT16U poX, INT16U poY, INT16U length, INT16U width, INT16U color);
 	
 	void drawLine(INT16U x0,INT16U y0,INT16U x1,INT16U y1,INT16U color);

libraries/esp8266/examples/RTCUserMemory/RTCUserMemory.ino

@@ -52,7 +52,7 @@ void setup() {
   }
 
   // Generate new data set for the struct
-  for (int i = 0; i < sizeof(rtcData.data); i++) {
+  for (size_t i = 0; i < sizeof(rtcData.data); i++) {
     rtcData.data[i] = random(0, 128);
   }
   // Update CRC32 of data
@@ -94,7 +94,7 @@ uint32_t calculateCRC32(const uint8_t *data, size_t length)
 void printMemory() {
   char buf[3];
   uint8_t *ptr = (uint8_t *)&rtcData;
-  for (int i = 0; i < sizeof(rtcData); i++) {
+  for (size_t i = 0; i < sizeof(rtcData); i++) {
     sprintf(buf, "%02X", ptr[i]);
     Serial.print(buf);
     if ((i + 1) % 32 == 0) {

tests/common.sh

@@ -40,7 +40,7 @@ function build_sketches()
     local build_arg=$3
     local build_dir=build.tmp
     mkdir -p $build_dir
-    local build_cmd="python tools/build.py -b generic -v -k -p $PWD/$build_dir $build_arg "
+    local build_cmd="python tools/build.py -b generic -v -w all -k -p $PWD/$build_dir $build_arg "
     local sketches=$(find $srcpath -name *.ino)
     print_size_info >size.log
     export ARDUINO_IDE_PATH=$arduino
@@ -50,7 +50,7 @@ function build_sketches()
         local sketchdirname=$(basename $sketchdir)
         local sketchname=$(basename $sketch)
         if [[ "${sketchdirname}.ino" != "$sketchname" ]]; then
-            echo "Skipping $sketch, beacause it is not the main sketch file";
+            echo "Skipping $sketch, because it is not the main sketch file";
             continue
         fi;
         if [[ -f "$sketchdir/.test.skip" ]]; then
@@ -68,6 +68,12 @@ function build_sketches()
             cat build.log
             set -e
             return $result
+        else
+            local warns=$( grep -c warning: build.log )
+            if [ $warns -ne 0 ]; then
+                echo "Warnings detected, log follows:"
+                cat build.log
+            fi
         fi
         rm build.log
         print_size_info $build_dir/*.elf >>size.log
@@ -97,6 +103,9 @@ function install_ide()
     mkdir esp8266com
     cd esp8266com
     ln -s $core_path esp8266
+    # Set custom warnings for all builds (i.e. could add -Wextra at some point)
+    echo "compiler.c.extra_flags=-Wall -Werror" > esp8266/platform.local.txt
+    echo "compiler.cpp.extra_flags=-Wall -Werror" >> esp8266/platform.local.txt
     cd esp8266/tools
     python get.py
     export PATH="$ide_path:$core_path/tools/xtensa-lx106-elf/bin:$PATH"