Check Print::write(byte) return and stop on fail

[earlephilhower]

The default Print::write(byte, count) method was continuing to send
bytes one-by-one even when a prior write returned 0. Because the buffer
pointer was incremented no matter success or fail, this leads to data
corruption as you'll not send some bytes in the middle and will then
send extra bytes past the end of the passed in buffer.

Because there's no concept of timeout, just stop on the first time
write(byte) fails and return the total bytes successfully written
and let the user worry about retrying or handling an error.

Found by @d-a-v as discussed on gitter.

[earlephilhower]

This is a small patch but has potentially large code exposure since it's in the Print:: base class. Appreciate lots of eyeballs on it and its interactions with the 10s of subclasses that implement it.

[devyte]

My single commentt: are there any examples that should be modified to implement correct error handling for this?

[earlephilhower]

Good question. Related to this specific code, I would say no because most examples only do Serial.write(a,b) which cannot, by construction, do anything but write every character out.

AFAICT, only the WiFi* and SPIFFS methods seem to have the chance to do partial ::writes. Serial.write (most common) will either always print out all chars, or always fail. For SPIFFS, a failed write probably means out of space, so there's generally not a way to recover (but an error would be nice). For WiFi, a retry could actually get the data to the other side (i.e. the write failure could be transient). EEPROM writes are going to be hard failures always since it's not actually writing to EEPROM but a RAM buffer this does not indicate a potential temporary flash issue).

Per my grep-fu, the following examples are not checking WiFi*.write() return values and should be fixed in another PR:

earle@server:~/Arduino/hardware/esp8266com/esp8266$ grep -r 'write('  | grep examples/ | grep ,
libraries/ESP8266HTTPClient/examples/StreamHttpClient/StreamHttpClient.ino:            USE_SERIAL.write(buff, c);
libraries/ESP8266WiFi/examples/WiFiTelnetToSerial/WiFiTelnetToSerial.ino:        serverClients[i].write(sbuf, len);
libraries/ESP8266WiFi/examples/NTPClient/NTPClient.ino:  udp.write(packetBuffer, NTP_PACKET_SIZE);
libraries/Ethernet/examples/udpntpclient_pedantic/UdpNtpClient_Pedantic.ino:  Udp.write((byte *)&packetBuffer, NTP_PACKET_SIZE);
libraries/Ethernet/examples/UdpNtpClient/UdpNtpClient.ino:  Udp.write(packetBuffer, NTP_PACKET_SIZE);
libraries/ESP8266WebServer/examples/WebUpdate/WebUpdate.ino:        if (Update.write(upload.buf, upload.currentSize) != upload.currentSize) {
libraries/ESP8266WebServer/examples/FSBrowser/FSBrowser.ino:      fsUploadFile.write(upload.buf, upload.currentSize);
libraries/ESP8266WebServer/examples/SDWebServer/SDWebServer.ino:      uploadFile.write(upload.buf, upload.currentSize);

[d-a-v]

My attention was caught by SD library. Code is quite tricky. What I could see is SdFile:: is inheriting Stream:: but defines write(void*,len) and not write(uint8_t*,len). This is worth further checking.
Should we do a reminder issue for this and @earlephilhower's above comment ?

[earlephilhower]

The SD libs are...interesting. Love this comment on SDFile::write():

 * \return For success write() returns the number of bytes written, always
 * \a nbyte.  If an error occurs, write() returns -1.  Possible errors
 * include write() is called before a file has been opened, write is called
 * for a read-only file, device is full, a corrupt file system or an I/O error.
....
actual code executed on error:

 writeErrorReturn:
  // return for write error
  //writeError = true;
  setWriteError();
  return 0;
}

The signatures are all messed up, too:

size_t SdFile::write(const char* str);
...but....
void SdFile::write_P(PGM_P str);