Skip to content

Conversation

@agkaminski
Copy link
Member

@agkaminski agkaminski commented Aug 23, 2023

DONE: RTOS-569

Description

Fixes phoenix-rtos/phoenix-rtos-project#181

Motivation and Context

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

How Has This Been Tested?

  • Already covered by automatic testing.
  • New test added: (add PR link here).
  • Tested by hand on: (ia32-generic-qemu).

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing linter checks and tests passed.
  • My changes generate no new compilation warnings for any of the targets.

Special treatment

  • This PR needs additional PRs to work (list the PRs, preferably in merge-order).
  • I will merge this PR by myself when appropriate.

@nalajcie
Copy link
Member

IMHO this should be implementation of strerror, perror might keep using strerror :)

@gerard5
Copy link
Contributor

gerard5 commented Aug 23, 2023

From my point of view, two implementations depending on the available platform resources (mem) should be considered. For example, after this change, each program that uses perror() e.g. psh increases by ca. 2KiB (imxrt, stm32). The earlier solution strerror() already implements a table for shorter error codes, and this is where the functionality should be extended, so that strerror() returns either a short description (for small targets, nommu) or a long error description for (zynq, ia32, 6ull), .. or depending on build config.

@agkaminski agkaminski force-pushed the agkaminski/rtos-569 branch from f9da6aa to 4dd2ecd Compare August 23, 2023 16:01
@agkaminski agkaminski changed the title perror: Add full implementation strerror, strerror_r, gai_strerror, perror: Add full implementation Aug 23, 2023
@agkaminski agkaminski requested a review from nalajcie August 23, 2023 16:03
@agkaminski
Copy link
Member Author

@gerard5 I've removed previous strerror string table which should somewhat reduce memory usage

@agkaminski agkaminski force-pushed the agkaminski/rtos-569 branch from 4dd2ecd to 55b0231 Compare August 23, 2023 16:06
@gerard5
Copy link
Contributor

gerard5 commented Aug 23, 2023

@gerard5 I've removed previous strerror string table which should somewhat reduce memory usage

It's still a bad bargain, now it costs 1.9KiB (compared to current master) for each program that use strerror(). It would be good to provide a choice at the project configuration stage whether a project developer want full error descriptions or abbreviated ones (targets with limited resources).

@agkaminski agkaminski force-pushed the agkaminski/rtos-569 branch from 55b0231 to 3fb185a Compare August 24, 2023 09:09
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.

libphoenix: perror implementation

5 participants