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

feat: add s2n_strerror_source API #4209

Merged
merged 4 commits into from
Sep 21, 2023
Merged

Conversation

camshaft
Copy link
Contributor

@camshaft camshaft commented Sep 19, 2023

Resolved issues:

Resolves #4208

Description of changes:

As noted in the issue, customers currently lack a good solution to get a short error message that just contains the filename and line number. This PR adds such an API.

Call-outs:

Luckily, this change doesn't actually increase the size of the final artifact at all. This is due to the fact that we use the same static string produce by _S2N_DEBUG_STR, just indexed into where the filename is. You can see that in an example godbolt: https://godbolt.org/z/b5rdY8rnn.

Testing:

I added a simple unit test showing that the output is just the file name.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@github-actions github-actions bot added the s2n-core team label Sep 19, 2023
@camshaft camshaft force-pushed the error-file-line branch 3 times, most recently from b511402 to 7bd50d5 Compare September 19, 2023 23:38
@camshaft camshaft force-pushed the error-file-line branch 3 times, most recently from 87b6250 to 622fc61 Compare September 20, 2023 21:04
@camshaft camshaft marked this pull request as ready for review September 20, 2023 21:40
@camshaft camshaft requested a review from dougch as a code owner September 20, 2023 21:40
@camshaft camshaft changed the title feat: add s2n_strerror_file_line API feat: add s2n_strerror_source API Sep 21, 2023
Co-authored-by: Sam Clark <[email protected]>
Copy link
Contributor

Choose a reason for hiding this comment

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

Not really related to your PR, but should this even be committed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably not...? No idea 😄

@camshaft camshaft requested a review from lrstewart September 21, 2023 22:21
@camshaft camshaft enabled auto-merge (squash) September 21, 2023 22:44
@camshaft camshaft merged commit b80c555 into aws:main Sep 21, 2023
@camshaft camshaft deleted the error-file-line branch September 22, 2023 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow s2n_strerror_debug output without unecessary filepath prefixed
3 participants