utils: continued result migration

[camshaft]

Please note that while we are transitioning from travis-ci to AWS CodeBuild, some tests are run on each platform. Non-AWS contributors will temporarily be unable to see CodeBuild results. We apologize for the inconvenience.

Resolved issues:

This is a continuation of the work started in #1872.

Description of changes:

s2n_ensure is introduced as a low-level definition for safety check helpers. This is intended to make it easier for static analyzers and verifiers to hook in and add additional checks.

s2n_safety has been updated to use the definitions provided by s2n_ensure. I also added a short description of what each macro does. A family of ENSURE macros are added to hopefully make the check macro names a little more consistent.

Several otherutils modules have been migrated to use s2n_result, notably:

• s2n_array
• s2n_asn1_time
• s2n_random
• s2n_rfc5952
• s2n_set
• s2n_timer

The interface change cause all caller locations to be updated.

Call-outs:

There is still quite a bit left to migrate but this changeset was already big enough :)

Testing:

It is unlikely that additional unit tests are needed beyond what is there, as most of the changes are caught by the compiler due to naming and type changes.

I had to update a couple of the SAW proofs to reflect the new function signatures.

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

[codecov-io]

Codecov Report

Merging #1891 into master will increase coverage by 1.04%.
The diff coverage is 88.80%.

@@            Coverage Diff             @@
##           master    #1891      +/-   ##
==========================================
+ Coverage   80.06%   81.10%   +1.04%     
==========================================
  Files         229      233       +4     
  Lines       17014    17312     +298     
==========================================
+ Hits        13622    14041     +419     
+ Misses       3392     3271     -121     

[danielsn]

Instead of int,

1. create a new typedef int POSIX_INT_ERRORCODE, and use that for external functions.
2. Add a check in "grep simple mistakes" that checks for any naked uses of int. I'd say that all ints should either be defined types: uint32_t, uint64_t, size_t, or they should be POSIX_INT_ERRORCODE. That makes it very clear what's going on.

[camshaft]

Instead of int,

1. create a new typedef int POSIX_INT_ERRORCODE, and use that for external functions.

2. Add a check in "grep simple mistakes" that checks for any naked uses of int. I'd say that all ints should either be defined types: uint32_t, uint64_t, size_t, or they should be POSIX_INT_ERRORCODE. That makes it very clear what's going on.

I think that's a great idea. We discussed offline and it's probably best to do it in another PR.

[codecov-commenter]

Codecov Report

Merging #1891 into master will increase coverage by 1.48%.
The diff coverage is 89.41%.

@@            Coverage Diff             @@
##           master    #1891      +/-   ##
==========================================
+ Coverage   79.38%   80.86%   +1.48%     
==========================================
  Files         229      231       +2     
  Lines       17152    17510     +358     
==========================================
+ Hits        13616    14160     +544     
+ Misses       3536     3350     -186     

[danielsn]

the array -> vec thing feels like a separate PR.

[danielsn]

Are we OK with this? Or should it be S2N_BAIL

[camshaft]

I wanted to be consistent with GUARD and not prefix it. The same question goes for ENSURE and friends.

Since they're all private macros I think we're ok to go without the prefix.

[danielsn]

We should really update the dev guide at the same time as the code changes.

[baldwinmatt]

all nits - the only thing i see is that we have uint32_t lengths for arrays, then iterate over it using an int.

[baldwinmatt]

woot! thanks for iterating and fixing so many things!