diff mbox series

[Openvpn-devel,v3] Enable a subset of -Wextra

Message ID 20250923140854.21766-1-gert@greenie.muc.de
State New
Headers show
Series [Openvpn-devel,v3] Enable a subset of -Wextra | expand

Commit Message

Gert Doering Sept. 23, 2025, 2:08 p.m. UTC 
From: Frank Lichtenheld <frank@lichtenheld.com>

- Includes fixes for
  - -Wimplicit-fallthrough=2
    (=3 is default but requires replacing all
     fallthrough comments)
  - -Wmissing-field-initializers
  - -Wold-style-declaration
- All other warnings that would need fixes are
  disabled for now.

Change-Id: I9ce664d073a4e6a6d433e9e6f986a5086dae8aa1
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1202
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1202
This mail reflects revision 3 of this Change.

Acked-by according to Gerrit (reflected above):
Gert Doering <gert@greenie.muc.de>

Comments

Gert Doering Sept. 23, 2025, 2:35 p.m. UTC | #1 
Stared at the changes, some are "huh, wat?" (like the const <-> static
reordering, but so what), one [[fallthrough]] was obviously forgotten
in commit fed37d0, and the struct assignment gets easier to read this
way, so double benefit :-) - wrt the CMakeFile change, GHA is fine with
it, and I don't have to understand it.

Your patch has been applied to the master branch.

commit 45eb7e1807060a927ff805b0d01fbb8cfbb772c8 (master)
Author: Frank Lichtenheld
Date:   Tue Sep 23 16:08:48 2025 +0200

     Enable a subset of -Wextra

     Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1202
     Message-Id: <20250923140854.21766-1-gert@greenie.muc.de>
     URL: https://sourceforge.net/p/openvpn/mailman/message/59237558/
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/CMakeLists.txt b/CMakeLists.txt
index f027b01..fdc0162 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -114,7 +114,14 @@ 
     check_and_add_compiler_flag(-Wstrict-prototypes StrictPrototypes)
     check_and_add_compiler_flag(-Wold-style-definition OldStyleDefinition)
     # We are not ready for this
-    #add_compile_options(-Wconversion -Wno-sign-conversion -Wsign-compare)
+    #add_compile_options(-Wconversion -Wno-sign-conversion)
+    add_compile_options(-Wextra -Wno-sign-compare -Wno-type-limits -Wno-unused-parameter)
+    # clang doesn't have the different levels but also doesn't include it in -Wextra
+    check_and_add_compiler_flag(-Wimplicit-fallthrough=2 GCCImplicitFallthrough)
+    if (WIN32)
+        # Not sure how to deal with GetProcAddress
+        add_compile_options(-Wno-cast-function-type)
+    endif ()
     if (USE_WERROR)
         add_compile_options(-Werror)
     endif ()
diff --git a/configure.ac b/configure.ac
index 38b14a1..a5485b0 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1398,6 +1398,13 @@ 
 ACL_CHECK_ADD_COMPILE_FLAGS([-Wstrict-prototypes])
 ACL_CHECK_ADD_COMPILE_FLAGS([-Wold-style-definition])
 ACL_CHECK_ADD_COMPILE_FLAGS([-Wall])
+ACL_CHECK_ADD_COMPILE_FLAGS([-Wextra -Wno-sign-compare -Wno-type-limits -Wno-unused-parameter])
+# clang doesn't have the different levels but also doesn't include it in -Wextra
+ACL_CHECK_ADD_COMPILE_FLAGS([-Wimplicit-fallthrough=2])
+if test "${WIN32}" = "yes"; then
+        # Not sure how to deal with GetProcAddress
+	ACL_CHECK_ADD_COMPILE_FLAGS([-Wno-cast-function-type])
+fi
 
 if test "${enable_pedantic}" = "yes"; then
 	enable_strict="yes"
diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c
index 59bf52b..0922326 100644
--- a/src/openvpn/misc.c
+++ b/src/openvpn/misc.c
@@ -733,7 +733,7 @@ 
                 {
                     return false;
                 }
-
+                /* Intentional [[fallthrough]]; */
             case 2:
                 /* after the '=', replace non-printable or shell meta with '_' */
                 if (!isprint(c) || isspace(c) || c == '$' || c == '(' || c == '`')
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 9618301..c213c4b 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -84,10 +84,10 @@ 
 
 #ifdef _WIN32
 
-const static GUID GUID_DEVCLASS_NET = {
+static const GUID GUID_DEVCLASS_NET = {
     0x4d36e972L, 0xe325, 0x11ce, { 0xbf, 0xc1, 0x08, 0x00, 0x2b, 0xe1, 0x03, 0x18 }
 };
-const static GUID GUID_DEVINTERFACE_NET = {
+static const GUID GUID_DEVINTERFACE_NET = {
     0xcac88484, 0x7515, 0x4c03, { 0x82, 0xe6, 0x71, 0xa8, 0x7a, 0xba, 0xc3, 0x61 }
 };
 
diff --git a/src/tapctl/tap.c b/src/tapctl/tap.c
index b633e77..9731602 100644
--- a/src/tapctl/tap.c
+++ b/src/tapctl/tap.c
@@ -40,11 +40,11 @@ 
 #endif
 
 
-const static GUID GUID_DEVCLASS_NET = {
+static const GUID GUID_DEVCLASS_NET = {
     0x4d36e972L, 0xe325, 0x11ce, { 0xbf, 0xc1, 0x08, 0x00, 0x2b, 0xe1, 0x03, 0x18 }
 };
 
-const static WCHAR szAdapterRegKeyPathTemplate[] =
+static const WCHAR szAdapterRegKeyPathTemplate[] =
     L"SYSTEM\\CurrentControlSet\\Control\\Network\\%ls\\%ls\\Connection";
 #define ADAPTER_REGKEY_PATH_MAX                                                                \
     (_countof(L"SYSTEM\\CurrentControlSet\\Control\\Network\\") - 1 + 38 + _countof(L"\\") - 1 \
diff --git a/tests/unit_tests/openvpn/test_pkt.c b/tests/unit_tests/openvpn/test_pkt.c
index b5211cd..1423d46 100644
--- a/tests/unit_tests/openvpn/test_pkt.c
+++ b/tests/unit_tests/openvpn/test_pkt.c
@@ -164,7 +164,7 @@ 
 init_tas_auth(int key_direction)
 {
     struct tls_auth_standalone tas = { 0 };
-    struct frame frame = { { .headroom = 200, .payload_size = 1400 }, 0 };
+    struct frame frame = { .buf = { .headroom = 200, .payload_size = 1400 }, 0 };
     tas.frame = frame;
 
     tas.tls_wrap.mode = TLS_WRAP_AUTH;
@@ -591,7 +591,7 @@ 
     enum first_packet_verdict verdict;
 
     tas.tls_wrap.mode = TLS_WRAP_NONE;
-    struct frame frame = { { .headroom = 200, .payload_size = 1400 }, 0 };
+    struct frame frame = { .buf = { .headroom = 200, .payload_size = 1400 }, 0 };
     tas.frame = frame;
     tas.workbuf = alloc_buf(1600);