-
Notifications
You must be signed in to change notification settings - Fork 122
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
Use _Static_assert in refcount_c11.c to support old compilers that don't support the macro static_assert #1789
Conversation
…n't support the macro static_assert
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1789 +/- ##
==========================================
- Coverage 78.33% 78.33% -0.01%
==========================================
Files 580 580
Lines 97256 97255 -1
Branches 13945 13946 +1
==========================================
- Hits 76187 76186 -1
- Misses 20445 20448 +3
+ Partials 624 621 -3 ☔ View full report in Codecov by Sentry. |
@@ -26,12 +26,12 @@ | |||
|
|||
|
|||
// See comment above the typedef of CRYPTO_refcount_t about these tests. | |||
static_assert(alignof(CRYPTO_refcount_t) == alignof(_Atomic CRYPTO_refcount_t), | |||
_Static_assert(alignof(CRYPTO_refcount_t) == alignof(_Atomic CRYPTO_refcount_t), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use OPENSSL_STATIC_ASSERT other places for this same purpose. It's defined here:
aws-lc/include/openssl/type_check.h
Lines 67 to 98 in 7d3101f
// Previously we defined |OPENSSL_STATIC_ASSERT| to use one of two keywords: | |
// |Static_assert| or |static_assert|. The latter was used if we were compiling | |
// a C++ translation unit or on Windows (excluding when using a Clang compiler). | |
// The former was used in other cases. However, these two keywords are not | |
// defined before C11. So, we can't rely on these when we want to be C99 | |
// compliant. If we at some point decide that we want to only be compliant with | |
// C11 (and up), we can reintroduce these keywords. Instead, use a method that | |
// is guaranteed to be C99 compliant and still give us an equivalent static | |
// assert mechanism. | |
// | |
// The solution below defines a struct type containing a bit field. | |
// The name of that type is |static_assertion_msg|. |msg| is a concatenation of | |
// a user-chosen error (which should be chosen with respect to actual assertion) | |
// and the line the assertion is defined. This should ensure name uniqueness. | |
// The width of the bit field is set to 1 or -1, depending on the evaluation of | |
// the boolean expression |cond|. If the condition is false, the width requested | |
// is -1, which is illegal and would cause the compiler to throw an error. | |
// | |
// An example of an error thrown during compilation: | |
// ``` | |
// error: negative width in bit-field | |
// 'static_assertion_at_line_913_error_is_AEAD_state_is_too_small' | |
// ``` | |
#define AWSLC_CONCAT(left, right) left##right | |
#define AWSLC_STATIC_ASSERT_DEFINE(cond, msg) typedef struct { \ | |
unsigned int AWSLC_CONCAT(static_assertion_, msg) : (cond) ? 1 : - 1; \ | |
} AWSLC_CONCAT(static_assertion_, msg) OPENSSL_UNUSED; | |
#define AWSLC_STATIC_ASSERT_ADD_LINE0(cond, suffix) AWSLC_STATIC_ASSERT_DEFINE(cond, AWSLC_CONCAT(at_line_, suffix)) | |
#define AWSLC_STATIC_ASSERT_ADD_LINE1(cond, line, suffix) AWSLC_STATIC_ASSERT_ADD_LINE0(cond, AWSLC_CONCAT(line, suffix)) | |
#define AWSLC_STATIC_ASSERT_ADD_LINE2(cond, suffix) AWSLC_STATIC_ASSERT_ADD_LINE1(cond, __LINE__, suffix) | |
#define AWSLC_STATIC_ASSERT_ADD_ERROR(cond, suffix) AWSLC_STATIC_ASSERT_ADD_LINE2(cond, AWSLC_CONCAT(_error_is_, suffix)) | |
#define OPENSSL_STATIC_ASSERT(cond, error) AWSLC_STATIC_ASSERT_ADD_ERROR(cond, error) |
#1791) …tandard version differences in refcount_c11.c ### Description of changes: Follow up from #1789 (comment) ### Testing: Tested locally and on old platforms, this is the same safety macro we use everywhere else in our code and is well supported. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license. --------- Co-authored-by: Justin W Smith <[email protected]>
Description of changes:
Some other old compilers don't support the convince macro
static_assert
but do support the C11 keyword_Static_assert
even though both should be in assert.h.Testing:
Built all old platforms internally.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.