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

refactor: switch s2n_blob to use S2N_RESULT #4448

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jmayclin
Copy link
Contributor

@jmayclin jmayclin commented Mar 4, 2024

Resolved issues:

chips away at #2425

Description of changes:

This PR switches s2n_blob methods to use s2n_result.

There are 5 manual changes

  • utils/s2n_blob.c: switch all implementations to use s2n_result with the appropriate guard changes
  • utils/s2n_blob.h: switch the header to match the implementation
  • tls/s2n_async_pkey.c:141: There was a manual comparison with S2N_SUCCESS, switched to use s2n_result_is_error
  • utils/s2n_mem.c:s2n_free(305): the return type is stored, so the type of that variable had to be changed
  • utils/s2n_mem.c:s2n_free_or_wipe(333): the return type is stored, so the type of that variable had to be change.

All other changes were made with the following sed commands.

find . -type f -exec sed -i 's/POSIX_GUARD(s2n_blob/POSIX_GUARD_RESULT(s2n_blob/g' {} +
find . -type f -exec sed -i 's/PTR_GUARD_POSIX(s2n_blob/PTR_GUARD_RESULT(s2n_blob/g' {} +
find . -type f -exec sed -i 's/RESULT_GUARD(s2n_blob/RESULT_GUARD(s2n_blob/g' {} +
find . -type f -exec sed -i 's/EXPECT_SUCCESS(s2n_blob/EXPECT_OK(s2n_blob/g' {} +

Call-outs:

Draft note: It would be lovely to make this a completely automated PR, but I don't see an easy way to do that and keep things compiling. ☹️

Testing:

All existing tests should pass.

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 Mar 4, 2024
Copy link

github-actions bot commented May 4, 2024

This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant