Fix for String memory corruption

[drmpf]

This patch fixes String memory corruption due to str +=str; and String(1.0,40); // and similar were result exceeds the local buf[33]

[CLAassistant]

All committers have signed the CLA.

[matthijskooijman]

Thanks for your contribution, sounds like useful things to fix indeed.

However, the development of the String class currently happens at https://github.com/arduino/ArduinoCore-API. The AVR core does note make use of the repository yet, but it should do so in the future, so any changes to the String class should be made there. Maybe you could check if this issue exists in that repo and maybe submit a fix there if it does?

As for the actual changes, some remarks:

• You add a fairly long copyright/changelog block, but it's generally considered best to leave such attribution to the git history. If every bugfix would add such a block, it would grow very big very soon.
• You really make three separate changes: Fix memory corruption, add a max number of decimals check and add handling of nan/inf/ovr. Ideally, these three changes would be in separate commits, making them easier to review and understand (now and later when looking through history).
• You added a memmove, but left the existing strcpy, so now memory is copied twice. I also do not understand the problem and solution here entirely, but I suspect this issue was already fixed here.
• There is a lot of code duplication for doing float/double to string conversion. This duplication already existed, but since it was only a single function call, that was not so much a problem before. With your change, there is a lot of duplicate code. This should probably be solved by letting the float version of the constructor call the double version of the constructor (should not be any slower anyway, since float-to-double conversion happens for dtostrf anyway and float and double are equal on AVR too). This change should ideally be another separate commit, probably applied before making any changes to the conversion (so the changes need to be made only once).

[drmpf]

Thanks for the pointer to the ArduinoCore-API. You are correct about the strcpy, bad merge on my part should have been commented out. Yes the fix you noted removes the need for my memmove The float/double to string conversion has a fix committed there also. So my change also may not needed except perhaps the nan/inf handling.