utils: add s2n_result implementation

[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.

Issue # (if available):
Part 1 for #1782 and #1783

Description of changes:

The goal of s2n_result is to provide a strongly-typed error signal value, which provides the compiler with enough information to catch bugs.

Historically, s2n has used int to signal errors. This has caused a few issues:

GUARD in a function returning integer types

There is no compiler error if GUARD(nested_call()); is used in a function that is meant to return integer type - not a error signal.

uint8_t s2n_answer_to_the_ultimate_question() {
  GUARD(s2n_sleep_for_years(7500000));
  return 42;
}

In this function we intended to return a uint8_t but used a GUARD which will return -1 if the call fails. This can lead to very subtle bugs.

GUARDing a function returning any integer type

There is no compiler error if GUARD(nested_call()); is used on a function that doesn't actually return an error signal

int s2n_deep_thought() {
  GUARD(s2n_answer_to_the_ultimate_question());
  return 0;
}

In this function we intended guard against a failure of s2n_answer_to_the_ultimate_question but that function doesn't actually return an error signal. Again, this can lead to sublte
bugs.

Ignored error signals

Without the warn_unused_result function attribute, the compiler provides no warning when forgetting to GUARD a function. Missing a GUARD can lead to subtle bugs.

int s2n_answer_to_the_ultimate_question() {
  s2n_sleep_for_years(7500000); // <- THIS SHOULD BE GUARDED!!!
  return 42;
}

Solution

s2n_result provides a newtype declaration, which is popular in languages like Haskell and Rust.

Functions that return S2N_RESULT are automatically marked with the warn_unused_result attribute, which ensures they are GUARDed.

Follow up

Over the next week I will gradually be moving functions to return S2N_RESULT instead of int.

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

[baldwinmatt]

Nice!

[zz85]

first contribution! one thing to note: functions in fuzz tests are unguarded, if we enforce compiler warnings, we could consider adding FUZZ_GUARD() macros.

[camshaft]

Ignoring an error signal, even in fuzz tests, is a bad idea. Having a guard macro that crashes might be the way to go there. But we can address that once I get to those.

[codecov-io]

Codecov Report

Merging #1872 into master will increase coverage by 0.00%.
The diff coverage is 97.26%.

@@           Coverage Diff           @@
##           master    #1872   +/-   ##
=======================================
  Coverage   81.44%   81.45%           
=======================================
  Files         226      228    +2     
  Lines       16905    16921   +16     
=======================================
+ Hits        13769    13783   +14     
- Misses       3136     3138    +2     

[colmmacc]

Type system based checking in C! Love this.

[zz85]

would be nice to add to the development/usage guide which macros should be used, and how to use them during the transition phase

GUARD_RESULT() - should use only with new s2n_result functions and returns s2n_result
GUARD_AS_RESULT() - guard old guard functions but returns new s2n_results
GUARD_AS_POSIX() - guard new s2n_result function but returns old guard int

[camshaft]

I'll add the guide in a separate PR

[baldwinmatt]

Lgtm! Looking forward to this!