Skip to content

Change uart_write() to return size when under control of GDBStub - #9326

Open
tomvaneyck wants to merge 1 commit into
esp8266:masterfrom
tomvaneyck:patch-1
Open

tomvaneyck wants to merge 1 commit into
esp8266:masterfrom
tomvaneyck:patch-1

Conversation

@tomvaneyck

Copy link
Copy Markdown

Description

When GDBStub has UART ISR control, uart_write() forwards the buffer to gdbstub_write(), which transmits all of it, but then returns 0:

size_t
uart_write(uart_t* uart, const char* buf, size_t size) {
[...]
if(gdbstub_has_uart_isr_control() && uart->uart_nr == UART0) {
    gdbstub_write(buf, size);
    return 0;
}
[...]
}

As a result, Serial.write(buf, size) reports that nothing was written even though every byte was sent.

Impact

Plain Serial.print() usage is unaffected because it ignores the return value. Code that relies on the return value breaks, notably Stream::SendGenericPeekBuffer(), which is used by Stream::sendAll() and HTTPClient::writeToStream(). Because write() returns 0, the loop never calls peekConsume() and never resets its timeout. It sends the same bytes from the start of the source buffer again on every iteration until the timeout expires. The serial output fills with repeated copies of the first few bytes, and HTTPClient::writeToStream(&Serial) fails with HTTPC_ERROR_READ_TIMEOUT (-11).

Reproduction

#include <GDBStub.h>
#include <StreamDev.h>

void setup() {
    Serial.begin(115200);
    gdbstub_init();

    String s;
    for (int i = 0; i < 100; i++) s += "Line " + String(i) + " abcdefghijklmnopqrstuvwxyz\n";

    size_t n = StreamConstPtr(s).sendAll(Serial);
    Serial.printf("\nsendAll() returned %u, expected %u\n", n, s.length());
}

void loop() {}
  • Expected: The text is printed once, and sendAll() returns s.length().
  • Actual: The start of the text is printed repeatedly for about 1 s (the default stream timeout), and sendAll() returns 0.

Fix

 if(gdbstub_has_uart_isr_control() && uart->uart_nr == UART0) {
     gdbstub_write(buf, size);
-    return 0;
+    return size;
 }

Disclaimer: I found the issue using claude, but checked the solution myself.

Change return value from 0 to size when gdbstub is active to follow function contract.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant