String ::remove, ::trim have overlapping strncpy/memcpy, undefined results per standard specs

[earlephilhower]

The ESP8266 Arduino port just discovered the following line in String::remove where strncpy is used with overlapping source and destination addresses (it's moving things backwards in the allocated buffer). See link for the valgrind warnings reported by our CI system.

ArduinoCore-avr/cores/arduino/WString.cpp

Lines 697 to 699 in b7c6076

	char *writeTo = buffer + index;
	len = len - count;
	strncpy(writeTo, buffer + index + count,len - index);

str*cpy is undefined when you overlap source and destination

While it's true that on the AVR, an 8-bit MCU, str*cpy will be implemented in a byte-wise manner, this code has and will probably be copied to 16- and 32-bit systems where they may use optimizations which give strange results occasionally due to the overlap. A simple replacement of the strcpy with a memmove in this line clears this error and provides desired behavior on all systems.

[earlephilhower]

Similar problem with the ::trim method:

ArduinoCore-avr/cores/arduino/WString.cpp

Lines 722 to 727 in b7c6076

	char *begin = buffer;
	while (isspace(*begin)) begin++;
	char *end = buffer + len - 1;
	while (isspace(*end) && end >= begin) end--;
	len = end + 1 - begin;
	if (begin > buffer) memcpy(buffer, begin, len);

Replacing memcpy with memmove on line 727 will fix this undefined usage on all systems.

[facchinm]

@earlephilhower wow, thanks for sharing, we'll investigate if it applies to other 32bit cores and apply the fixes. Thanks a lot!

[facchinm]

Applied in arduino/ArduinoCore-API@b99609e , now it's a matter of moving cores to API asap 🙂