Skip to content

drivers/at: at_readline_stop_at_str fix remaining buffer length#21195

Merged
fabian18 merged 1 commit intoRIOT-OS:masterfrom
fabian18:pr/at_rx_fix_off_by_one_bug
Feb 7, 2025
Merged

drivers/at: at_readline_stop_at_str fix remaining buffer length#21195
fabian18 merged 1 commit intoRIOT-OS:masterfrom
fabian18:pr/at_rx_fix_off_by_one_bug

Conversation

@fabian18
Copy link
Copy Markdown
Contributor

@fabian18 fabian18 commented Feb 5, 2025

Contribution description

Fixes a tiny bug in the at driver that causes a return of -ENOBUFS although the received response fits in the buffer.
It happens when the end-of-line sequence is at the last position before \0.

RIOT/drivers/at/at.c

Lines 518 to 528 in c20ce69

if ((size_t)(resp_pos - resp_buf) >= strlen(AT_RECV_EOL)) {
char *const eol_begin = resp_pos - strlen(AT_RECV_EOL);
if (strcmp(eol_begin, AT_RECV_EOL) == 0) {
if (keep_eol) {
break;
}
*eol_begin = '\0';
resp_pos -= strlen(AT_RECV_EOL);
break;
}
}

When the while loop is exited because '\n' was received at the last position before the last \0, the len variable was already decremented. So the -ENOBUFS condition triggers.

RIOT/drivers/at/at.c

Lines 538 to 540 in c20ce69

if (len <= 1) {
return -ENOBUFS;
}

Testing procedure

Configure a URC and send a sequence over the UART via pyterm to the microcontroller.
For example "012345678901234567890123456789\n\0" for rp_buf_size = 32.

Issues/PRs references

@github-actions github-actions bot added the Area: drivers Area: Device drivers label Feb 5, 2025
@kfessel
Copy link
Copy Markdown
Contributor

kfessel commented Feb 6, 2025

Is the keep_eol case missing a len-- with this PR?

eol not replaced by \0 -> missing space for that

--
read another time the \0 is allready there ( memset)

Copy link
Copy Markdown
Contributor

@kfessel kfessel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (len == 1) is ok since 0 will never be reached due to while stopping at 1 ( last loop 2)

this last byte already is a \0 due to memset

@fabian18 fabian18 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Feb 6, 2025
@riot-ci
Copy link
Copy Markdown

riot-ci commented Feb 6, 2025

Murdock results

✔️ PASSED

5633a19 drivers/at: at_readline_stop_at_str fix remaining buffer length

Success Failures Total Runtime
10271 0 10271 11m:30s

Artifacts

@fabian18 fabian18 added this pull request to the merge queue Feb 7, 2025
Merged via the queue into RIOT-OS:master with commit 7e33118 Feb 7, 2025
@mguetschow mguetschow added this to the Release 2025.04 milestone Apr 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants