cmake: fix generation of options.h #7546
Conversation
|
Can one of the admins verify this patch? |
|
Thank you @oltolm , I will review. Contributor agreement on file. Okay to test. |
dgarske
assigned oltolm, JacobBarthelmeh, SparkiDev and dgarske and unassigned oltolm and dgarske
|
Hi @oltolm, What is this PR actually fixing? It seems to just change the way options.h is generated to use configure_file and adds additional maintenance. I checked and the current method for generating options.h works fine and doesn't require maintaining a list of macros. Are there benefits to using configure_file? Thanks, |
|
Yes. This is from the official documentation:
If the |
|
Hi @oltolm , Thank you for the feedback. I'll review this with the engineering team tomorrow and made a decision. Thanks, |
CMakeLists.txt
Outdated
| endif() | ||
| endforeach() | ||
|
|
||
| configure_file(${CMAKE_CURRENT_SOURCE_DIR}/wolfssl/options.h.in ${OPTION_FILE}) |
There was a problem hiding this comment.
Please add a comment here about needing to update the file for new options. Also add a comment in cmake/README.md.
|
Hi @oltolm , We discussed this internally provided a few items to adjust. Then this PR should be acceptable. Thank you again for your work on this improvement. Thank you, |
…e about adding new options.
tests/api.c
Outdated
| @@ -146,6 +146,7 @@ | |||
| #endif | |||
| #include <wolfssl/error-ssl.h> | |||
|
|
|||
| #include <stdint.h> | |||
There was a problem hiding this comment.
Without the fix I get this on Windows with GCC 14.1.
FAILED: CMakeFiles/unit_test.dir/tests/api.c.obj
C:\msys64\mingw64\bin\gcc.exe -DECC_SHAMIR -DECC_TIMING_RESISTANT -DGCM_TABLE_4BIT -DHAVE_AESGCM -DHAVE_CHACHA -DHAVE_CONFIG_H -DHAVE_DH_DEFAULT_PARAMS -DHAVE_ECC -DHAVE_ENCRYPT_THEN_MAC -DHAVE_EXTENDED_MASTER -DHAVE_FFDHE_2048 -DHAVE_HASHDRBG -DHAVE_HKDF -DHAVE_ONE_TIME_AUTH -DHAVE_POLY1305 -DHAVE_PTHREAD -DHAVE_SNI -DHAVE_SUPPORTED_CURVES -DHAVE_THREAD_LS -DHAVE_TLS_EXTENSIONS -DHAVE___UINT128_T -DNO_DES3 -DNO_DES3_TLS_SUITES -DNO_DSA -DNO_MD4 -DNO_PSK -DNO_RC4 -DTFM_ECC256 -DTFM_TIMING_RESISTANT -DWC_NO_ASYNC_THREADING -DWC_RSA_BLINDING -DWC_RSA_PSS -DWOLFSSL_BASE64_ENCODE -DWOLFSSL_DLL -DWOLFSSL_IGNORE_FILE_WARN -DWOLFSSL_NO_SHAKE128 -DWOLFSSL_NO_SHAKE256 -DWOLFSSL_PSS_LONG_SALT -DWOLFSSL_SHA224 -DWOLFSSL_SHA3 -DWOLFSSL_SHA384 -DWOLFSSL_SHA512 -DWOLFSSL_SYS_CA_CERTS -DWOLFSSL_TLS13 -DWOLFSSL_USE_ALIGN -DWOLFSSL_X86_64_BUILD -D_POSIX_THREADS -IC:/src/wolfssl/build -IC:/src/wolfssl -Wall -g -DNO_MAIN_DRIVER -MD -MT CMakeFiles/unit_test.dir/tests/api.c.obj -MF CMakeFiles\unit_test.dir\tests\api.c.obj.d -o CMakeFiles/unit_test.dir/tests/api.c.obj -c C:/src/wolfssl/tests/api.c
C:/src/wolfssl/tests/api.c: In function 'test_read_write_hs':
C:/src/wolfssl/tests/api.c:71565:5: error: unknown type name 'uint8_t'
71565 | uint8_t test_buffer[16];
| ^~~~~~~
C:/src/wolfssl/tests/api.c:5519:1: note: 'uint8_t' is defined in header '<stdint.h>'; this is probably fixable by adding '#include <stdint.h>'
5518 | #include <wolfssl/openssl/pem.h>
+++ |+#include <stdint.h>
5519 | /*----------------------------------------------------------------------------*
There was a problem hiding this comment.
Thank you @oltolm . I went ahead and removed the uses of uint8_t. The reason we weren't seeing this is because configure sets HAVE_UINT_PTR that includes stdint.h.
tests/api.c
Outdated
| @@ -146,6 +146,7 @@ | |||
| #endif | |||
| #include <wolfssl/error-ssl.h> | |||
|
|
|||
| #include <stdint.h> | |||
There was a problem hiding this comment.
Thank you @oltolm . I went ahead and removed the uses of uint8_t. The reason we weren't seeing this is because configure sets HAVE_UINT_PTR that includes stdint.h.
cmake: fix generation of options.h
Description
I fixed the way
options.his generated. It is now being generated properly usingconfigure_file. Fixes #4489.Testing
I built wolfssl with CMake.