dd25e2b breaks some integer to string conversion functions, for example ultoa.

[rtrbt]

Description:

The fix for #5045 in commit dd25e2b tries to fix an issue with multiple definitions for reverse in stdlib_noniso.c and esp_dsp.h. However this breaks some integer to string conversion functions, that use the reverse function, at least ultoa, as well as the String constructors using those functions.

Expected output is 123456 (twice), but 654321 is printed instead.

If I change the guard in line 33 to #if 1 it works as expected.

arduino-esp32/cores/esp32/stdlib_noniso.c

Lines 33 to 47 in 4ada3f5

	#if !CONFIG_DSP_ANSI && !CONFIG_DSP_OPTIMIZED
	void reverse(char* begin, char* end) {
	char *is = begin;
	char *ie = end - 1;
	while(is < ie) {
	char tmp = *ie;
	*ie = *is;
	*is = tmp;
	++is;
	--ie;
	}
	}
	#else
	void reverse(char* begin, char* end);
	#endif

Sketch:

void setup() {
  // Wait for serial monitor to attach
  delay(3000);
  Serial.begin(115200);

  // Using string constructor
  String s = String(123456ul, 10);
  Serial.println(s);

  // Using ultoa directly
  char buf[10] = {0};
  ultoa(123456ul, buf, 10);
  Serial.println(buf);
}

void loop() {}

[me-no-dev]

In that case, maybe changing it to static could do the trick. Could you please try to remove the guards and rename the function to _reverse, then use the new name throughout stdlib_noniso?

[rtrbt]

Both versions (static void reverse and void _reverse) seem to work, but I don't have checked if one of the variants collides with definitions in esp_dsp.h. However I would not expect that esp_dsp.h is pulled in by esp_system.h (as it seems to be a separate library?) so marking the function static should be the way to go.

[stale]

[STALE_SET] This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

[VojtechBartoska]

Hello, I'm closing this issue as there is already merged PR. Thanks for your contribution @rtrbt!