[M] Change in openvpn[master]: Make cppcheck run pass on top of current master

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/+/1869?usp=email

to review the following change.


Change subject: Make cppcheck run pass on top of current master
......................................................................

Make cppcheck run pass on top of current master

* Update cppcheck-suppressions to be in sync with
  current code
* Suppress some additional findings that we have
  patches for in the queue but which are not merged,
  yet
* Add some additional fixes for categories we had
  fixed but for which new occurences crept in

The goal is to have one commit that allows us to
enable a cppcheck run in buildbot to keep it
up to date going forward.

Change-Id: Id56fbfa4767346995eaa05ac95c7a20fbba99b2d
Signed-off-by: Frank Lichtenheld <[email protected]>
---
M dev-tools/cppcheck-suppression
M src/openvpn/mudp.c
M src/openvpn/multi.c
M src/openvpn/multi_io.c
M src/openvpn/openvpn.c
M src/openvpn/ssl_mbedtls.c
M tests/unit_tests/openvpn/test_pkt.c
7 files changed, 42 insertions(+), 23 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/69/1869/1

diff --git a/dev-tools/cppcheck-suppression b/dev-tools/cppcheck-suppression
index 900e03c..a585513 100644
--- a/dev-tools/cppcheck-suppression
+++ b/dev-tools/cppcheck-suppression
@@ -5,6 +5,10 @@
 constParameterPointer
 invalidPrintfArgType_sint
 invalidPrintfArgType_uint
+redundantAssignment
+redundantInitialization
+unreadVariable
+unusedFunction
 usleepCalled
 variableScope
 # We have a lot of library includes, not all of them are really required,
@@ -24,15 +28,21 @@
 localtimeCalled
 strtokCalled
 # FP: posix.cfg claims suseconds_t is unsigned for some reason
-unsignedLessThanZero:src/openvpn/otime.h:235
+unsignedLessThanZero:src/openvpn/otime.h:148
 # IGN: multi code does weird things with pointers to local variables...
-autoVariables:src/openvpn/multi.c:4177
-autoVariables:src/openvpn/multi_io.c:280
+autoVariables:src/openvpn/multi.c:4232
+autoVariables:src/openvpn/multi_io.c:324
 # IGN: the code header = 0 | (OPCODE << P_OPCODE_SHIFT) is used intentionally
 badBitmaskCheck:src/openvpn/mudp.c
 badBitmaskCheck:tests/unit_tests/openvpn/test_pkt.c
 # IGN: we store integers in pointers
 CastAddressToIntegerAtReturn:src/openvpn/multi.c
+# IGN: Windows specific (unsigned long == unsigned int)
+compareValueOutOfTypeRangeError:src/openvpn/ssl_verify.c:928
+# FP: cppcheck seems to have wrong signature of DeviceIoControl()
+constVariablePointer:src/openvpn/dco_win.c
+# IGN: test_networking code would break with iproute2 but that is prevented
+ctunullpointer:src/openvpn/networking_iproute2.c
 # IGN: event code uses a pointer to store integers
 intToPointerCast:src/openvpn/forward.c
 intToPointerCast:src/openvpn/multi_io.c
@@ -40,7 +50,7 @@
 # FP: constant but differs between platforms
 knownConditionTrueFalse:src/openvpn/error.h:380
 knownConditionTrueFalse:src/openvpn/fdmisc.c:80
-knownConditionTrueFalse:src/openvpn/lladdr.c:65
+knownConditionTrueFalse:src/openvpn/lladdr.c:64
 knownConditionTrueFalse:src/openvpn/platform.c
 # FP: code needs to accomodate many different defines
 knownConditionTrueFalse:src/openvpn/event.c:1139
@@ -48,13 +58,13 @@
 # FP: dco_win support has "false" stubs
 knownConditionTrueFalse:src/openvpn/forward.c
 knownConditionTrueFalse:src/openvpn/init.c
-knownConditionTrueFalse:src/openvpn/multi_io.c:163
+knownConditionTrueFalse:src/openvpn/multi_io.c:197
 # FP: cppcheck thinks that some functions always return true, but they don't
 knownConditionTrueFalse:src/openvpn/misc.c:97
 knownConditionTrueFalse:src/openvpn/sig.h:116
 # FP: cert_uri_supported is a wrapper around defines, so it's
 # always constant but differs depending on OpenSSL version
-knownConditionTrueFalse:src/openvpn/ssl_openssl.c:1332
+knownConditionTrueFalse:src/openvpn/ssl_openssl.c:1260
 # FP: cppcheck doesn't understand that the function changes szErrMessage
 knownConditionTrueFalse:src/tapctl/main.c:704
 knownConditionTrueFalse:src/openvpnmsica/dllmain.c:164
@@ -65,12 +75,18 @@
 # FP: eventmsg.h is not built on Unix
 missingInclude:src/openvpnserv/common.c:25
 # IGN: strlen(NULL) is not nice code, but seems to work
-nullPointerRedundantCheck:src/openvpn/init.c:299
+nullPointerRedundantCheck:src/openvpn/init.c:301
 # FP: cppcheck doesn't understand ZeroMemory
 redundantAssignment:src/openvpnserv/interactive.c:203
+# FP: cppcheck doesn't know the NLA macros
+redundantInitialization:src/openvpn/dco_linux.c
 # IGN: We reuse the same variable name due to macro usage
-shadowVariable:src/openvpn/options.c:2580
-shadowVariable:src/openvpn/options.c:2598
+shadowVariable:src/openvpn/options.c:1948
+shadowVariable:src/openvpn/options.c:1966
+# IGN: sure this is theoretically undefined, but works
+shiftNegativeLHS:tests/unit_tests/openvpn/test_schedule.c:183
+# FP: fun:tls_crypt_v2_wrap_unwrap_invalid: cppcheck is confused
+syntaxError:tests/unit_tests/openvpn/test_tls_crypt.c:684
 # FP: this file is never compiled on _WIN32
 umaskCalled:tests/unit_tests/openvpn/test_pkcs11.c
 # FP: yes, t_prev is unitialized, but t_prev_len is 0, so that's handled
@@ -78,13 +94,13 @@
 # FP: yes, parm is unitialized, but parm_len is 0, so that's handled
 uninitvar:src/openvpn/options_parse.c:148
 # FP: uninit is fine when it is a return parameter
-ctuuninitvar:src/openvpn/crypto_mbedtls_legacy.c:698
-uninitvar:src/openvpnserv/interactive.c:1935
+ctuuninitvar:src/openvpn/crypto_mbedtls_legacy.c:690
+uninitvar:src/openvpnserv/interactive.c:2775
 uninitvar:src/tapctl/main.c:566
 # FP: we added a check but cppcheck is not convinced
 uninitvar:src/openvpnserv/interactive.c:2667
 # FP: weird parse error, the macro is fine in the rest of the file
-unknownMacro:src/openvpnserv/interactive.c:3488
+unknownMacro:src/openvpnserv/interactive.c:3596
 # FP: cppcheck doesn't account for short-circuiting
 unreadVariable:src/openvpn/manage.c:682
 unusedFunction:src/openvpn/siphash_reference.c
@@ -101,8 +117,10 @@
 # FP: doesn't account for --wrap
 unusedFunction:tests/unit_tests/openvpn/test_tls_crypt.c
 unusedFunction:/usr/include/*
+# FP: cppcheck doesn't know the NLA macros
+unusedLabel:src/openvpn/dco_linux.c
 # IGN: old code that is difficult to test (MSG_ERRQUEUE), ignore for now
-unusedStructMember:src/openvpn/mtu.c:281
+unusedStructMember:src/openvpn/mtu.c:289
 # FP: used implictly by NL macros
 unusedStructMember:src/openvpn/networking_sitnl.c
 # IGN: keep explanatory fields in test data
@@ -111,3 +129,5 @@
 variableScope:src/openvpn/networking_sitnl.c:1390
 # IGN: nicer to keep the "variable" earlier
 variableScope:src/openvpnserv/interactive.c:2687
+# FP: fun:platform_create_temp_file: cppcheck is confused
+wrongPrintfScanfArgNum:src/openvpn/platform.c:553
diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
index 6d9c181..de3d467 100644
--- a/src/openvpn/mudp.c
+++ b/src/openvpn/mudp.c
@@ -324,7 +324,7 @@
 {
     struct multi_instance *mi = NULL;
 
-    uint8_t *ptr = BPTR(&m->top.c2.buf);
+    const uint8_t *ptr = BPTR(&m->top.c2.buf);
     uint8_t op = ptr[0] >> P_OPCODE_SHIFT;
     bool v2 = (op == P_DATA_V2) && (m->top.c2.buf.len >= (1 + 3));
     bool peer_id_disabled = false;
@@ -376,7 +376,7 @@
         return NULL;
     }
 
-    uint8_t *ptr = BPTR(&m->top.c2.buf);
+    const uint8_t *ptr = BPTR(&m->top.c2.buf);
     uint8_t op = ptr[0] >> P_OPCODE_SHIFT;
 
     struct mroute_addr real = { 0 };
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 60a5148..74939bf 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -3095,8 +3095,8 @@
 
     struct multi_instance *ex_mi = he->value;
 
-    struct tls_multi *m1 = mi->context.c2.tls_multi;
-    struct tls_multi *m2 = ex_mi->context.c2.tls_multi;
+    const struct tls_multi *m1 = mi->context.c2.tls_multi;
+    const struct tls_multi *m2 = ex_mi->context.c2.tls_multi;
 
     struct gc_arena gc = gc_new();
     int ret = false;
diff --git a/src/openvpn/multi_io.c b/src/openvpn/multi_io.c
index d8cc708..3604684 100644
--- a/src/openvpn/multi_io.c
+++ b/src/openvpn/multi_io.c
@@ -191,12 +191,12 @@
 int
 multi_io_wait(struct multi_context *m)
 {
-    int status, i;
+    int status;
     unsigned int *persistent = &m->multi_io->tun_rwflags;
 
     if (!tuntap_is_dco_win(m->top.c1.tuntap))
     {
-        for (i = 0; i < m->top.c1.link_sockets_num; i++)
+        for (int i = 0; i < m->top.c1.link_sockets_num; i++)
         {
             socket_set_listen_persistent(m->top.c2.link_sockets[i], m->multi_io->es,
                                          &m->top.c2.link_sockets[i]->ev_arg);
diff --git a/src/openvpn/openvpn.c b/src/openvpn/openvpn.c
index 7d35195..24d6bb6 100644
--- a/src/openvpn/openvpn.c
+++ b/src/openvpn/openvpn.c
@@ -32,7 +32,6 @@
 #include "win32.h"
 #include "options_show.h"
 #include "platform.h"
-#include "string.h"
 
 #include "memdbg.h"
 
diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
index faf5a27..c92c423 100644
--- a/src/openvpn/ssl_mbedtls.c
+++ b/src/openvpn/ssl_mbedtls.c
@@ -286,7 +286,7 @@
 void
 tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers)
 {
-    char *tmp_ciphers, *tmp_ciphers_orig, *token;
+    char *tmp_ciphers, *tmp_ciphers_orig;
 
     if (NULL == ciphers)
     {
@@ -306,7 +306,7 @@
     char *lasts = NULL;
     tmp_ciphers_orig = tmp_ciphers = string_alloc(ciphers, NULL);
 
-    token = strtok_r(tmp_ciphers, ":", &lasts);
+    const char *token = strtok_r(tmp_ciphers, ":", &lasts);
     while (token)
     {
         ctx->allowed_ciphers[i] = mbedtls_ssl_get_ciphersuite_id(tls_translate_cipher_name(token));
diff --git a/tests/unit_tests/openvpn/test_pkt.c b/tests/unit_tests/openvpn/test_pkt.c
index 5ec6781..a732c2b 100644
--- a/tests/unit_tests/openvpn/test_pkt.c
+++ b/tests/unit_tests/openvpn/test_pkt.c
@@ -562,7 +562,7 @@
 static void
 test_calc_session_id_hmac_static(void **ut_state)
 {
-    uint8_t key[SIPHASH_KEY_SIZE] = { 1, 2, 3, 0 };
+    const uint8_t key[SIPHASH_KEY_SIZE] = { 1, 2, 3, 0 };
     static const int handwindow = 100;
 
     struct openvpn_sockaddr addr = { 0 };

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

Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Id56fbfa4767346995eaa05ac95c7a20fbba99b2d
Gerrit-Change-Number: 1869
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>

_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel