HardwareSerial::write(long/int) is incorrect/dangerous!

[cider101]

any value above 255 will just be cut off -

inline size_t write(unsigned long n) { return write((uint8_t)n); }
inline size_t write(long n) { return write((uint8_t)n); }
inline size_t write(unsigned int n) { return write((uint8_t)n); }
inline size_t write(int n) { return write((uint8_t)n); }

please remove or fix the incorrect implementations!

[matthijskooijman]

Took me a bit of searching (your bug reports are a bit concise), but I found the code here:
https://github.com/arduino/Arduino/blob/master/hardware/arduino/cores/arduino/HardwareSerial.h#L63-L66

The Print class only defines write(uint8_t), but for some reason HardwareSerial also defines write for other types. I'm not so sure why this is needed - I think the compiler will handle type conversion automatically in this case.

In any case, you're misunderstanding the purpose of write(): If you call it with an integer, it will write a single byte with that integer value - it is up to you to make sure the integer is between 0 and 255. If you want to print a full number, then use the print() method instead. If you want to write a bigger number in binary, you'll have to split it up into different bytes manually.

[cider101]

Thx for your answer. I'll try to be a more verbose in the future. I also fixed the subject of this report with the correct class name.

I fully understand the semantics of write() & print(). The problem here is, that the "long/int" versions of print versions are not just plain wrong but also very hard to find/debug why the numbers are "cut-off". (it' didn't happend to me - just found them by coincidence). Without those overloaded methods, the compiler will give you at least a warning about the "downcast" (or is it an error?). But with those overladed members, everthing seems to work on compile time.
Generally, i preferre "syntax-checks" over semantic-checks...meaning everything which can be "forbidden" at compile time should be handled that way...

[matthijskooijman]

@sandeepmistry, I'm not sure this really belongs in API. AFAICT from the comments, the Print class is ok, it is just the AVR HardwareSerial class that adds these possibly problematic (and at least unneeded, I think) overloads (maybe other cores too, I did not check).