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

utils: add safety_macros codegen script #2423

Merged
merged 5 commits into from
Mar 3, 2021
Merged

Conversation

camshaft
Copy link
Contributor

@camshaft camshaft commented Nov 30, 2020

This change continues the S2N_RESULT migration. In this phase, we add a codegen script to consistently generate all safety macros.

This also changes the macros to explicitly state the context in which it should be used. For example, if a safety macro is used in a function that returns an int (i.e. POSIX), safety calls would include POSIX_GUARD, POSIX_ENSURE, etc. If the function returns S2N_RESULT, calls should be prefixed with RESULT_ (e.g. RESULT_GUARD, RESULT_ENSURE). You can see a list of all of the calls in the auto-generated documentation.

Callouts

  • One thing to note is the RESULT_ prefix is temporary. After this PR is merged and other existng PRs apply the codemod script, we will have another PR that removes the RESULT_ prefix and makes S2N_RESULT the preferred error signal return type. Without an intermediate phase, the codemod script will not be idempotent, since all of the prefix-less calls will be ambiguous.

  • Another point is that all of the existing safety macros are unmodified. After applying the codemod script from utils: add safety codemod script #2339, the existing safety macros will be removed in favor of the new naming convention.

Rendered documentation

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

@codecov-io
Copy link

codecov-io commented Nov 30, 2020

Codecov Report

Merging #2423 (040ee91) into main (cf32218) will increase coverage by 0.03%.
The diff coverage is 98.43%.

@@            Coverage Diff             @@
##             main    #2423      +/-   ##
==========================================
+ Coverage   81.96%   81.99%   +0.03%     
==========================================
  Files         271      272       +1     
  Lines       18676    18726      +50     
==========================================
+ Hits        15307    15354      +47     
- Misses       3369     3372       +3     

Impacted file tree graph

utils/s2n_safety_macros.h Show resolved Hide resolved
docs/SAFETY-MACROS.md Outdated Show resolved Hide resolved
docs/SAFETY-MACROS.md Outdated Show resolved Hide resolved
utils/s2n_ensure.c Show resolved Hide resolved
error/s2n_errno.c Show resolved Hide resolved
tests/testlib/s2n_test_certs.c Show resolved Hide resolved
scripts/s2n_safety_macros.py Show resolved Hide resolved
@camshaft camshaft merged commit 6648c66 into aws:main Mar 3, 2021
@camshaft camshaft deleted the safety-macros branch March 3, 2021 04:19
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.

None yet

6 participants