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

Use _Static_assert in refcount_c11.c to support old compilers that don't support the macro static_assert #1789

Merged
merged 1 commit into from
Aug 22, 2024

Conversation

andrewhop
Copy link
Contributor

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.

@andrewhop andrewhop requested a review from a team as a code owner August 21, 2024 23:09
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.33%. Comparing base (7d3101f) to head (6cc8758).

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.
📢 Have feedback on the report? Share it here.

@@ -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),
Copy link
Contributor

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:

// 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)

@smittals2 smittals2 self-requested a review August 22, 2024 01:40
@andrewhop andrewhop merged commit 2f18797 into aws:main Aug 22, 2024
105 of 106 checks passed
justsmth added a commit that referenced this pull request Aug 23, 2024
#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 <103147162+justsmth@users.noreply.github.com>
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.

5 participants