Attention is currently required from: plaisthos.

Hello plaisthos,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1207?usp=email

to review the following change.


Change subject: Enable -Wtype-limits by default (via -Wextra)
......................................................................

Enable -Wtype-limits by default (via -Wextra)

Removes a few smaller instances:
 - Fix return type check for socket() on Windows/Unixy
 - Ignore a few instances related to WSAWaitForMultipleEvents.
   The compiler says the check is currently useless, but
   we follow the API documentation.
 - Fix one check that assumed peer_id was signed.

Change-Id: Iaabddb6f81cd94863291b193aae9d384a8f9d871
Signed-off-by: Frank Lichtenheld <[email protected]>
---
M CMakeLists.txt
M configure.ac
M src/openvpn/event.c
M src/openvpn/init.c
M src/openvpn/socket.c
5 files changed, 17 insertions(+), 5 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/07/1207/1

diff --git a/CMakeLists.txt b/CMakeLists.txt
index fdc0162..e4e0b51 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -115,7 +115,7 @@
     check_and_add_compiler_flag(-Wold-style-definition OldStyleDefinition)
     # We are not ready for this
     #add_compile_options(-Wconversion -Wno-sign-conversion)
-    add_compile_options(-Wextra -Wno-sign-compare -Wno-type-limits 
-Wno-unused-parameter)
+    add_compile_options(-Wextra -Wno-sign-compare -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)
diff --git a/configure.ac b/configure.ac
index c2feeea..8fc27c3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1398,7 +1398,7 @@
 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])
+ACL_CHECK_ADD_COMPILE_FLAGS([-Wextra -Wno-sign-compare -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
diff --git a/src/openvpn/event.c b/src/openvpn/event.c
index 581bdbb..77f859f 100644
--- a/src/openvpn/event.c
+++ b/src/openvpn/event.c
@@ -400,6 +400,13 @@
     }
 #endif

+    /* WSA_WAIT_EVENT_0 == 0 but the API documentation is written in a way
+       that doesn't guarantee that. So we make useless checks. */
+#if defined(__GNUC__)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wtype-limits"
+#endif
+
     /*
      * First poll our event list with 0 timeout
      */
@@ -464,6 +471,9 @@
             return -1;
         }
     }
+#if defined(__GNUC__)
+#pragma GCC diagnostic pop
+#endif
 }

 static struct event_set *
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 0d7a2ec..62b8138 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2724,7 +2724,7 @@
     /* Ensure that for epoch data format is only enabled if also data v2
      * is enabled */
     bool epoch_data = (c->options.imported_protocol_flags & 
CO_EPOCH_DATA_KEY_FORMAT);
-    bool datav2_enabled = (c->options.peer_id >= 0 && c->options.peer_id < 
MAX_PEER_ID);
+    bool datav2_enabled = c->options.peer_id < MAX_PEER_ID;

     if (epoch_data && !datav2_enabled)
     {
diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
index 94f16c7..9839357 100644
--- a/src/openvpn/socket.c
+++ b/src/openvpn/socket.c
@@ -572,7 +572,8 @@
     ASSERT(addrinfo);
     ASSERT(addrinfo->ai_socktype == SOCK_STREAM);

-    if ((sd = socket(addrinfo->ai_family, addrinfo->ai_socktype, 
addrinfo->ai_protocol)) < 0)
+    if ((sd = socket(addrinfo->ai_family, addrinfo->ai_socktype, 
addrinfo->ai_protocol))
+        != SOCKET_UNDEFINED)
     {
         msg(M_ERR, "Cannot create TCP socket");
     }
@@ -603,7 +604,8 @@
     ASSERT(addrinfo);
     ASSERT(addrinfo->ai_socktype == SOCK_DGRAM);

-    if ((sd = socket(addrinfo->ai_family, addrinfo->ai_socktype, 
addrinfo->ai_protocol)) < 0)
+    if ((sd = socket(addrinfo->ai_family, addrinfo->ai_socktype, 
addrinfo->ai_protocol))
+        != SOCKET_UNDEFINED)
     {
         msg(M_ERR, "UDP: Cannot create UDP/UDP6 socket");
     }

--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1207?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Iaabddb6f81cd94863291b193aae9d384a8f9d871
Gerrit-Change-Number: 1207
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
Gerrit-MessageType: newchange
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to