[Openvpn-devel] Change include order for tests

Message ID 20240212132522.125903-1-juliusz@wolfssl.com
State Accepted
Headers show
Series [Openvpn-devel] Change include order for tests | expand

Commit Message

Juliusz Sosinowicz Feb. 12, 2024, 1:25 p.m. UTC
Including "ssl.h" conflicts with the wolfSSL ssl.h header file. The openvpn/src directory needs to be included before include/wolfssl. include/wolfssl needs to be included so that openvpn can pick up wolfSSL compatibility headers instead of OpenSSL headers without changing the paths.

src/openvpn/Makefile.am does not need to be modified because AM_CPPFLAGS is placed before AM_CFLAGS in the output Makefile.

Signed-off-by: Juliusz Sosinowicz <juliusz@wolfssl.com>
---
 tests/unit_tests/openvpn/Makefile.am | 62 +++++++++++++++-------------
 1 file changed, 34 insertions(+), 28 deletions(-)

Comments

Arne Schwabe Feb. 12, 2024, 1:47 p.m. UTC | #1
Am 12.02.24 um 14:25 schrieb Juliusz Sosinowicz:
> Including "ssl.h" conflicts with the wolfSSL ssl.h header file. The openvpn/src directory needs to be included before include/wolfssl. include/wolfssl needs to be included so that openvpn can pick up wolfSSL compatibility headers instead of OpenSSL headers without changing the paths.
> 
> src/openvpn/Makefile.am does not need to be modified because AM_CPPFLAGS is placed before AM_CFLAGS in the output Makefile.
> 


That looks reasonable and having test includes in a more similar order 
than our normal includes is also a good thing.

Acked-By: Arne Schwabe <arne@rfc2549.org>
Gert Doering Feb. 12, 2024, 5:10 p.m. UTC | #2
Thanks for this updated patch, which fixes the issue in a much nicer
way.  I'm not sure if you observe the problem in release/2.6 as well
- if yes, I need a release/2.6-specific patch for that (as half the
new test drivers are not in that branch).

Quick test with an in-tree build on FreeBSD and GHA builds passes fine.

Your patch has been applied to the master branch.

commit 54475711eb119f6fbb263880fca08d4b10df752a
Author: Juliusz Sosinowicz
Date:   Mon Feb 12 14:25:22 2024 +0100

     Change include order for tests

     Signed-off-by: Juliusz Sosinowicz <juliusz@wolfssl.com>
     Acked-by: Arne Schwabe <arne@rfc2549.org>
     Message-Id: <20240212132522.125903-1-juliusz@wolfssl.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28229.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
Juliusz Sosinowicz Feb. 12, 2024, 6:11 p.m. UTC | #3
Thanks Gert. I see that our tests for the "release/2.6" branch are 
passing. This was discovered by testing against the master branch so I 
don't think this patch needs backporting.

Sincerely
Juliusz Sosinowicz

On 12/02/2024 18:10, Gert Doering wrote:
> Thanks for this updated patch, which fixes the issue in a much nicer
> way.  I'm not sure if you observe the problem in release/2.6 as well
> - if yes, I need a release/2.6-specific patch for that (as half the
> new test drivers are not in that branch).
>
> Quick test with an in-tree build on FreeBSD and GHA builds passes fine.
>
> Your patch has been applied to the master branch.
>
> commit 54475711eb119f6fbb263880fca08d4b10df752a
> Author: Juliusz Sosinowicz
> Date:   Mon Feb 12 14:25:22 2024 +0100
>
>       Change include order for tests
>
>       Signed-off-by: Juliusz Sosinowicz <juliusz@wolfssl.com>
>       Acked-by: Arne Schwabe <arne@rfc2549.org>
>       Message-Id: <20240212132522.125903-1-juliusz@wolfssl.com>
>       URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28229.html
>       Signed-off-by: Gert Doering <gert@greenie.muc.de>
>
>
> --
> kind regards,
>
> Gert Doering
>

Patch

diff --git a/tests/unit_tests/openvpn/Makefile.am b/tests/unit_tests/openvpn/Makefile.am
index ce6f8127..a4e6235f 100644
--- a/tests/unit_tests/openvpn/Makefile.am
+++ b/tests/unit_tests/openvpn/Makefile.am
@@ -41,7 +41,7 @@  if HAVE_SITNL
 check_PROGRAMS += networking_testdriver
 endif
 
-argv_testdriver_CFLAGS  = @TEST_CFLAGS@ -I$(top_srcdir)/src/openvpn -I$(top_srcdir)/src/compat
+argv_testdriver_CFLAGS  = -I$(top_srcdir)/src/openvpn -I$(top_srcdir)/src/compat @TEST_CFLAGS@
 argv_testdriver_LDFLAGS = @TEST_LDFLAGS@ -L$(top_srcdir)/src/openvpn -Wl,--wrap=parse_line
 argv_testdriver_SOURCES = test_argv.c mock_msg.c mock_msg.h \
 	mock_get_random.c \
@@ -50,15 +50,16 @@  argv_testdriver_SOURCES = test_argv.c mock_msg.c mock_msg.h \
 	$(top_srcdir)/src/openvpn/win32-util.c \
 	$(top_srcdir)/src/openvpn/argv.c
 
-buffer_testdriver_CFLAGS  = @TEST_CFLAGS@ -I$(top_srcdir)/src/openvpn -I$(top_srcdir)/src/compat
+buffer_testdriver_CFLAGS  = -I$(top_srcdir)/src/openvpn -I$(top_srcdir)/src/compat @TEST_CFLAGS@
 buffer_testdriver_LDFLAGS = @TEST_LDFLAGS@ -L$(top_srcdir)/src/openvpn -Wl,--wrap=parse_line
 buffer_testdriver_SOURCES = test_buffer.c mock_msg.c mock_msg.h \
 	mock_get_random.c \
 	$(top_srcdir)/src/openvpn/win32-util.c \
 	$(top_srcdir)/src/openvpn/platform.c
 
-crypto_testdriver_CFLAGS  = @TEST_CFLAGS@ \
-	-I$(top_srcdir)/include -I$(top_srcdir)/src/compat -I$(top_srcdir)/src/openvpn
+crypto_testdriver_CFLAGS  = \
+	-I$(top_srcdir)/include -I$(top_srcdir)/src/compat -I$(top_srcdir)/src/openvpn \
+	@TEST_CFLAGS@
 crypto_testdriver_LDFLAGS = @TEST_LDFLAGS@
 crypto_testdriver_SOURCES = test_crypto.c mock_msg.c mock_msg.h \
 	$(top_srcdir)/src/openvpn/buffer.c \
@@ -72,8 +73,9 @@  crypto_testdriver_SOURCES = test_crypto.c mock_msg.c mock_msg.h \
 	$(top_srcdir)/src/openvpn/win32-util.c \
 	$(top_srcdir)/src/openvpn/mss.c
 
-ssl_testdriver_CFLAGS  = @TEST_CFLAGS@ \
-	-I$(top_srcdir)/include -I$(top_srcdir)/src/compat -I$(top_srcdir)/src/openvpn
+ssl_testdriver_CFLAGS  = \
+	-I$(top_srcdir)/include -I$(top_srcdir)/src/compat -I$(top_srcdir)/src/openvpn \
+	@TEST_CFLAGS@
 ssl_testdriver_LDFLAGS = @TEST_LDFLAGS@  $(OPTIONAL_CRYPTO_LIBS)
 ssl_testdriver_SOURCES = test_ssl.c mock_msg.c mock_msg.h \
 	mock_management.c mock_ssl_dependencies.c mock_win32_execve.c \
@@ -106,8 +108,9 @@  if WIN32
 ssl_testdriver_LDADD =  -lcrypt32 -lncrypt -lfwpuclnt -liphlpapi -lws2_32
 endif
 
-packet_id_testdriver_CFLAGS  = @TEST_CFLAGS@ \
-	-I$(top_srcdir)/include -I$(top_srcdir)/src/compat -I$(top_srcdir)/src/openvpn
+packet_id_testdriver_CFLAGS  = \
+	-I$(top_srcdir)/include -I$(top_srcdir)/src/compat -I$(top_srcdir)/src/openvpn \
+	@TEST_CFLAGS@
 packet_id_testdriver_LDFLAGS = @TEST_LDFLAGS@
 packet_id_testdriver_SOURCES = test_packet_id.c mock_msg.c mock_msg.h \
 	mock_get_random.c \
@@ -119,8 +122,9 @@  packet_id_testdriver_SOURCES = test_packet_id.c mock_msg.c mock_msg.h \
 	$(top_srcdir)/src/openvpn/win32-util.c \
 	$(top_srcdir)/src/openvpn/session_id.c
 
-pkt_testdriver_CFLAGS  = @TEST_CFLAGS@ \
-	-I$(top_srcdir)/include -I$(top_srcdir)/src/compat -I$(top_srcdir)/src/openvpn
+pkt_testdriver_CFLAGS  = \
+	-I$(top_srcdir)/include -I$(top_srcdir)/src/compat -I$(top_srcdir)/src/openvpn \
+	@TEST_CFLAGS@
 pkt_testdriver_LDFLAGS = @TEST_LDFLAGS@
 pkt_testdriver_SOURCES = test_pkt.c mock_msg.c mock_msg.h mock_win32_execve.c \
 	$(top_srcdir)/src/openvpn/argv.c \
@@ -141,8 +145,9 @@  pkt_testdriver_SOURCES = test_pkt.c mock_msg.c mock_msg.h mock_win32_execve.c \
 	$(top_srcdir)/src/openvpn/tls_crypt.c
 
 if !WIN32
-tls_crypt_testdriver_CFLAGS  = @TEST_CFLAGS@ \
-	-I$(top_srcdir)/include -I$(top_srcdir)/src/compat -I$(top_srcdir)/src/openvpn
+tls_crypt_testdriver_CFLAGS  = \
+	-I$(top_srcdir)/include -I$(top_srcdir)/src/compat -I$(top_srcdir)/src/openvpn \
+	@TEST_CFLAGS@
 tls_crypt_testdriver_LDFLAGS = @TEST_LDFLAGS@ \
 	-Wl,--wrap=buffer_read_from_file \
 	-Wl,--wrap=buffer_write_file \
@@ -164,9 +169,9 @@  tls_crypt_testdriver_SOURCES = test_tls_crypt.c mock_msg.c mock_msg.h \
 endif
 
 if HAVE_SITNL
-networking_testdriver_CFLAGS = @TEST_CFLAGS@ \
+networking_testdriver_CFLAGS = \
 	-I$(top_srcdir)/include -I$(top_srcdir)/src/compat -I$(top_srcdir)/src/openvpn \
-	$(OPTIONAL_CRYPTO_CFLAGS)
+	@TEST_CFLAGS@ $(OPTIONAL_CRYPTO_CFLAGS)
 networking_testdriver_LDFLAGS = @TEST_LDFLAGS@ -L$(top_srcdir)/src/openvpn \
 	$(OPTIONAL_CRYPTO_LIBS)
 networking_testdriver_SOURCES = test_networking.c mock_msg.c \
@@ -180,9 +185,9 @@  networking_testdriver_SOURCES = test_networking.c mock_msg.c \
 	$(top_srcdir)/src/openvpn/platform.c
 endif
 
-provider_testdriver_CFLAGS  = @TEST_CFLAGS@ \
+provider_testdriver_CFLAGS  = \
 	-I$(top_srcdir)/include -I$(top_srcdir)/src/compat -I$(top_srcdir)/src/openvpn \
-	$(OPTIONAL_CRYPTO_CFLAGS)
+	@TEST_CFLAGS@ $(OPTIONAL_CRYPTO_CFLAGS)
 provider_testdriver_LDFLAGS = @TEST_LDFLAGS@ \
 	$(OPTIONAL_CRYPTO_LIBS)
 
@@ -196,9 +201,9 @@  provider_testdriver_SOURCES = test_provider.c mock_msg.c \
 	$(top_srcdir)/src/openvpn/platform.c
 
 if WIN32
-cryptoapi_testdriver_CFLAGS  = @TEST_CFLAGS@ \
+cryptoapi_testdriver_CFLAGS  = \
 	-I$(top_srcdir)/include -I$(top_srcdir)/src/compat -I$(top_srcdir)/src/openvpn \
-	$(OPTIONAL_CRYPTO_CFLAGS)
+	@TEST_CFLAGS@ $(OPTIONAL_CRYPTO_CFLAGS)
 cryptoapi_testdriver_LDFLAGS = @TEST_LDFLAGS@ \
 	$(OPTIONAL_CRYPTO_LIBS) -lcrypt32 -lncrypt
 cryptoapi_testdriver_SOURCES = test_cryptoapi.c mock_msg.c \
@@ -214,9 +219,9 @@  endif
 
 if HAVE_SOFTHSM2
 if !WIN32
-pkcs11_testdriver_CFLAGS  = @TEST_CFLAGS@ \
+pkcs11_testdriver_CFLAGS  = \
 	-I$(top_srcdir)/include -I$(top_srcdir)/src/compat -I$(top_srcdir)/src/openvpn \
-	$(OPTIONAL_CRYPTO_CFLAGS)
+	@TEST_CFLAGS@ $(OPTIONAL_CRYPTO_CFLAGS)
 pkcs11_testdriver_LDFLAGS = @TEST_LDFLAGS@ \
 	$(OPTIONAL_CRYPTO_LIBS)
 pkcs11_testdriver_SOURCES = test_pkcs11.c mock_msg.c \
@@ -235,9 +240,9 @@  pkcs11_testdriver_SOURCES = test_pkcs11.c mock_msg.c \
 endif
 endif
 
-auth_token_testdriver_CFLAGS  = @TEST_CFLAGS@ \
+auth_token_testdriver_CFLAGS  = \
 	-I$(top_srcdir)/include -I$(top_srcdir)/src/compat -I$(top_srcdir)/src/openvpn \
-	$(OPTIONAL_CRYPTO_CFLAGS)
+	@TEST_CFLAGS@ $(OPTIONAL_CRYPTO_CFLAGS)
 auth_token_testdriver_LDFLAGS = @TEST_LDFLAGS@ \
 	$(OPTIONAL_CRYPTO_LIBS)
 
@@ -253,8 +258,9 @@  auth_token_testdriver_SOURCES = test_auth_token.c mock_msg.c \
 	$(top_srcdir)/src/openvpn/base64.c
 
 
-user_pass_testdriver_CFLAGS  = @TEST_CFLAGS@ \
-	-I$(top_srcdir)/include -I$(top_srcdir)/src/compat -I$(top_srcdir)/src/openvpn
+user_pass_testdriver_CFLAGS  = \
+	-I$(top_srcdir)/include -I$(top_srcdir)/src/compat -I$(top_srcdir)/src/openvpn \
+	@TEST_CFLAGS@
 user_pass_testdriver_LDFLAGS = @TEST_LDFLAGS@
 
 user_pass_testdriver_SOURCES = test_user_pass.c mock_msg.c \
@@ -269,9 +275,9 @@  user_pass_testdriver_SOURCES = test_user_pass.c mock_msg.c \
 	$(top_srcdir)/src/openvpn/base64.c
 
 
-ncp_testdriver_CFLAGS  = @TEST_CFLAGS@ \
+ncp_testdriver_CFLAGS  = \
 	-I$(top_srcdir)/include -I$(top_srcdir)/src/compat -I$(top_srcdir)/src/openvpn \
-	$(OPTIONAL_CRYPTO_CFLAGS)
+	@TEST_CFLAGS@ $(OPTIONAL_CRYPTO_CFLAGS)
 ncp_testdriver_LDFLAGS = @TEST_LDFLAGS@ \
 	$(OPTIONAL_CRYPTO_LIBS)
 
@@ -287,9 +293,9 @@  ncp_testdriver_SOURCES = test_ncp.c mock_msg.c \
 	$(top_srcdir)/src/compat/compat-strsep.c \
 	$(top_srcdir)/src/openvpn/ssl_util.c
 
-misc_testdriver_CFLAGS  = @TEST_CFLAGS@ \
+misc_testdriver_CFLAGS  = \
 	-I$(top_srcdir)/include -I$(top_srcdir)/src/compat -I$(top_srcdir)/src/openvpn \
-	-DSOURCEDIR=\"$(top_srcdir)\"
+	-DSOURCEDIR=\"$(top_srcdir)\" @TEST_CFLAGS@
 
 misc_testdriver_LDFLAGS = @TEST_LDFLAGS@