Predeclare enums in base.h Including ssl.h is quite a chunk of code and #defines, so we've tried to limit its spread internally in the interests of code hygine given that we have a multi-billion-line repo. However, header files that mention enums from ssl.h currently need to include ssl.h. For example, your class may have static class member functions intended to be callbacks, and they need to be class members because they'll call other private methods. C cannot predeclare enums, but C++ can if you explicitly type them. Sadly C doesn't support explicit types. So option one is to move the enums into base.h. That works, but the enums properly live in ssl.h and reading the header file is a lot clearer if you don't have to jump around to see all the pieces. So option two (this change) is to explicitly type and predelcare the enums in base.h for C++ only. The worry now is that C and C++ might disagree about the type of the enums. However, this has already happened: at least for |ssl_private_key_result_t|, g++ thinks that it's an |int| (without any explicit type) and gcc thinks that it's an |unsigned|. At least they're the same length, I guess? So, to make sure that this doesn't slip any more, this change also adds |ssl_test_c.c| which tests that C views the enums as having the same size as an |int|, at least. Change-Id: I8248583ec997021f8226d5a798609f6afc96dac4 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/35664 Reviewed-by: Adam Langley <agl@google.com> Reviewed-by: David Benjamin <davidben@google.com> Commit-Queue: Adam Langley <agl@google.com>
diff --git a/include/openssl/base.h b/include/openssl/base.h index 7fe232f..41e9305 100644 --- a/include/openssl/base.h +++ b/include/openssl/base.h
@@ -291,6 +291,23 @@ #endif #endif // OPENSSL_ASM_INCOMPATIBLE +#if defined(__cplusplus) +// enums can be predeclared, but only in C++ and only if given an explicit type. +// C doesn't support setting an explicit type for enums thus a #define is used +// to do this only for C++. However, the ABI type between C and C++ need to have +// equal sizes, which is confirmed in a unittest. +#define BORINGSSL_ENUM_INT : int +enum ssl_private_key_result_t BORINGSSL_ENUM_INT; +enum ssl_ticket_aead_result_t BORINGSSL_ENUM_INT; +enum ssl_verify_result_t BORINGSSL_ENUM_INT; +enum ssl_encryption_level_t BORINGSSL_ENUM_INT; +enum ssl_renegotiate_mode_t BORINGSSL_ENUM_INT; +enum ssl_select_cert_result_t BORINGSSL_ENUM_INT; +enum ssl_select_cert_result_t BORINGSSL_ENUM_INT; +#else +#define BORINGSSL_ENUM_INT +#endif + // CRYPTO_THREADID is a dummy value. typedef int CRYPTO_THREADID;
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 4240c29..8e02084 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h
@@ -1160,7 +1160,7 @@ // Custom private keys. -enum ssl_private_key_result_t { +enum ssl_private_key_result_t BORINGSSL_ENUM_INT { ssl_private_key_success, ssl_private_key_retry, ssl_private_key_failure, @@ -2111,7 +2111,7 @@ // ssl_ticket_aead_result_t enumerates the possible results from decrypting a // ticket with an |SSL_TICKET_AEAD_METHOD|. -enum ssl_ticket_aead_result_t { +enum ssl_ticket_aead_result_t BORINGSSL_ENUM_INT { // ssl_ticket_aead_success indicates that the ticket was successfully // decrypted. ssl_ticket_aead_success, @@ -2285,7 +2285,7 @@ int (*callback)(int ok, X509_STORE_CTX *store_ctx)); -enum ssl_verify_result_t { +enum ssl_verify_result_t BORINGSSL_ENUM_INT { ssl_verify_ok, ssl_verify_invalid, ssl_verify_retry, @@ -3135,7 +3135,7 @@ // ssl_encryption_level_t represents a specific QUIC encryption level used to // transmit handshake messages. -enum ssl_encryption_level_t { +enum ssl_encryption_level_t BORINGSSL_ENUM_INT { ssl_encryption_initial = 0, ssl_encryption_early_data, ssl_encryption_handshake, @@ -3522,7 +3522,7 @@ // such as HTTP/1.1, and not others, such as HTTP/2. OPENSSL_EXPORT void SSL_set_shed_handshake_config(SSL *ssl, int enable); -enum ssl_renegotiate_mode_t { +enum ssl_renegotiate_mode_t BORINGSSL_ENUM_INT { ssl_renegotiate_never = 0, ssl_renegotiate_once, ssl_renegotiate_freely, @@ -3620,7 +3620,7 @@ // ssl_select_cert_result_t enumerates the possible results from selecting a // certificate with |select_certificate_cb|. -enum ssl_select_cert_result_t { +enum ssl_select_cert_result_t BORINGSSL_ENUM_INT { // ssl_select_cert_success indicates that the certificate selection was // successful. ssl_select_cert_success = 1,
diff --git a/ssl/CMakeLists.txt b/ssl/CMakeLists.txt index dc89dca..0fb532e 100644 --- a/ssl/CMakeLists.txt +++ b/ssl/CMakeLists.txt
@@ -50,6 +50,7 @@ span_test.cc ssl_test.cc + ssl_c_test.c $<TARGET_OBJECTS:boringssl_gtest_main> )
diff --git a/ssl/ssl_c_test.c b/ssl/ssl_c_test.c new file mode 100644 index 0000000..02f8655 --- /dev/null +++ b/ssl/ssl_c_test.c
@@ -0,0 +1,15 @@ +#include <openssl/ssl.h> + +int BORINGSSL_enum_c_type_test(void); + +int BORINGSSL_enum_c_type_test(void) { +#if defined(__cplusplus) +#error "This is testing the behaviour of the C compiler." +#error "It's pointless to build it in C++ mode." +#endif + + // In C++, the enums in ssl.h are explicitly typed as ints to allow them to + // be predeclared. This function confirms that the C compiler believes them + // to be the same size as ints. They may differ in signedness, however. + return sizeof(enum ssl_private_key_result_t) == sizeof(int); +}
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc index d01b649..0ff1867 100644 --- a/ssl/ssl_test.cc +++ b/ssl/ssl_test.cc
@@ -5263,6 +5263,15 @@ EXPECT_EQ(SSL_process_quic_post_handshake(client_.get()), 0); } +extern "C" { +int BORINGSSL_enum_c_type_test(void); +} + +TEST(SSLTest, EnumTypes) { + EXPECT_EQ(sizeof(int), sizeof(ssl_private_key_result_t)); + EXPECT_EQ(1, BORINGSSL_enum_c_type_test()); +} + // TODO(davidben): Convert this file to GTest properly. TEST(SSLTest, AllTests) { if (!TestSSL_SESSIONEncoding(kOpenSSLSession) ||