Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sys/tiny_strerror: add tiny_strerror_minimal #18768

Merged
merged 1 commit into from
Oct 19, 2022

Conversation

benpicco
Copy link
Contributor

Contribution description

tiny_strerror is still quite heavy (1200 bytes on Cortex-M4), so add a more minimal version that only prints the error code.

This allows to use the same code and have pretty error messages if ROM is ample, but still have the option to strip it down easily.

Testing procedure

Issues/PRs references

@benpicco benpicco requested a review from maribu October 18, 2022 20:06
@github-actions github-actions bot added Area: build system Area: Build system Area: sys Area: System labels Oct 18, 2022
Copy link
Member

@maribu maribu left a comment

Choose a reason for hiding this comment

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

ACK. Nice idea. Please address inline comment and squash before merging.

sys/tiny_strerror/tiny_strerror.c Outdated Show resolved Hide resolved
@@ -117,6 +118,12 @@ const char *tiny_strerror(int errnum)
errnum = -errnum;
}

if (IS_USED(MODULE_TINY_STRERROR_MINIMAL)) {
static char buf[4];
Copy link
Member

Choose a reason for hiding this comment

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

This is not thread-safe. I don't think this is much of an issue, as in few cases more than one thread cares about user-friendly error messages. And if so they would have to interrupt each other during error reporting. And even then likely they report errors via stdio, which gets garbled anyway when two threads concurrently write to it.

Still, I think this is something we need to point out clearly with a big fat @warning.

@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Oct 18, 2022
@riot-ci
Copy link

riot-ci commented Oct 18, 2022

Murdock results

✔️ PASSED

8553c8f sys/tiny_strerror: add tiny_strerror_minimal

Success Failures Total Runtime
1991 0 1991 07m:03s

Artifacts

This only reflects a subset of all builds from https://ci-prod.riot-os.org. Please refer to https://ci.riot-os.org for a complete build for now.

@maribu maribu enabled auto-merge October 19, 2022 05:11
@maribu maribu merged commit 1c00ce2 into RIOT-OS:master Oct 19, 2022
@benpicco benpicco deleted the tiny_strerror_minimal branch October 19, 2022 12:45
@kaspar030 kaspar030 added this to the Release 2023.01 milestone Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: build system Area: Build system Area: sys Area: System 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