refactor: enforce stuffer return check #4399
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- add no-unused-result to fuzz makefile There are many cases in our s2n-fuzz tests where we deliberately do not assert on the success of certain operations because it is expect in happy path fuzzing that they will fail. Rather than manully casting everything to avoid these warnings, we disable this error for fuzz tests.
jmayclin
changed the title
lrstewart
reviewed
Mar 1, 2024
- manually add stuffer wipe assertion
- return EIO in case of error
We set s2n_errno, no the actual system errno. And when we're mocking out the system functionality we have to deal with both.
lrstewart
approved these changes
Mar 1, 2024
goatgoose
approved these changes
Mar 1, 2024
Co-authored-by: Sam Clark <3758302+goatgoose@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolved issues:
Resolves #4391
Description of changes:
s2n-tls has strict standard for checking the results of functions, but we rely on contributors remembering to check all of these values. This PR adds "must use" attributes to emit warnings when these conventions are not followed, and also fixes all of those warning that currently exist in our codebase.
Call-outs:
Ideally we'd switch to S2N_RESULT, but that is a much larger effort. This PR instead aims for incremental progress.
We can't force MUST_USE attributed on methods that are used for DEFER_CLEANUP.
Testing:
I added
-Werrorflags and then fixed thing until it compiled. This will be validated by the validator and asan builds that run with stricted checks.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.