arduino-libraries / ArduinoHttpClient

Arduino HTTP Client library
282 stars 170 forks source link

Fix incorrect return value, resulting in compilation error, being there for 8 years! #49

Closed forGGe closed 5 years ago

forGGe commented 5 years ago

Error in question:

ArduinoHttpClient/src/HttpClient.h: In member function 'virtual void HttpClient::flush()':
ArduinoHttpClient/src/HttpClient.h:310:50: error: return-statement with a value, in function returning 'void' [-fpermissive]

Now this one really pissed me off. It is not the error itself, but a fact that this was ignored from the first time it was uploaded to GitHub:

$ git blame -L 309,311 -- src/HttpClient.h

^6e63262 HttpClient.h (amcewen 2011-05-14 14:42:26 +0100 309)     virtual int peek() { return iClient->peek(); };
^6e63262 HttpClient.h (amcewen 2011-05-14 14:42:26 +0100 310)     virtual void flush() { return iClient->flush(); };

...AND ESP team was knowing about it and no one give a single bit of effort porting that change into mainstream. Despite hundreds of people using arduino ecosystem and this lib.

It looks even more retarded , that avr-gcc let it pass trough.

Notes for future: please add CI here with grain of static analysis tool. It is not that hard.

facchinm commented 5 years ago

ESP team arbitrarily changed the signature of flush . This is something that should be resolved on the ESP side (or at least discussed by everyone before approaching such a breaking change) Original API: https://github.com/arduino/ArduinoCore-API/blob/master/api/Client.h#L36 Related issue: https://github.com/arduino-libraries/ArduinoHttpClient/issues/48