diff mbox series

[Openvpn-devel,v4] dev-tools: Fix run-cppcheck to cover more code

Message ID 20260607170713.4980-1-gert@greenie.muc.de
State New
Headers show
Series [Openvpn-devel,v4] dev-tools: Fix run-cppcheck to cover more code | expand

Commit Message

Gert Doering June 7, 2026, 5:07 p.m. UTC 
From: Frank Lichtenheld <frank@lichtenheld.com>

Massively improve how we call cppcheck to cover
more code and identify more issues.

When specifying any -D argument all other defines
are ignored unless --force or --max-configs is
specified as well. I mistakenly assumed that this
was covered by --check-level=exhaustive. We need
to try finding a value for --max-configs so that
cppcheck doesn't spend hours scanning options.c

Add a library cfg for our code which for now
- identifies some printf-style functions
- adds some common macro defines

Use existing libraries.

Add a second call to cppcheck to separate the
Windows and Unixy code scans. This avoids some
very non-sensical define combinations.

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

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/+/1665
This mail reflects revision 4 of this Change.

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

Patch

diff --git a/dev-tools/cppcheck-suppression b/dev-tools/cppcheck-suppression
index 439cbb1..2e3350b 100644
--- a/dev-tools/cppcheck-suppression
+++ b/dev-tools/cppcheck-suppression
@@ -5,6 +5,9 @@ 
 constParameterPointer
 constVariable
 constVariablePointer
+invalidPrintfArgType_sint
+invalidPrintfArgType_uint
+usleepCalled
 variableScope
 # We have a lot of library includes, not all of them are really required,
 # so ignore them
@@ -16,29 +19,44 @@ 
 # These are specific false-positives (FP) or ignored (IGN) issues
 # We might want to move some of them to inline-suppression to avoid
 # the static line-numbers
+# IGN: posix.cfg: We are not threadsafe
+getgrnamCalled
+getpwnamCalled
+getservbynameCalled
+localtimeCalled
+strtokCalled
+# FP: posix.cfg claims suseconds_t is unsigned for some reason
+unsignedLessThanZero:src/openvpn/otime.h:235
 # IGN: multi code does weird things with pointers to local variables...
 autoVariables:src/openvpn/multi.c:4177
 autoVariables:src/openvpn/multi_io.c:280
 # 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: event code uses a pointer to store integers
-intToPointerCast:src/openvpn/multi_io.c
 intToPointerCast:src/openvpn/forward.c
-# FP: crt_error is always true on Unix, but not Windows
+intToPointerCast:src/openvpn/multi_io.c
+intToPointerCast:src/openvpn/ps.c
+# 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/platform.c
 # FP: code needs to accomodate many different defines
+knownConditionTrueFalse:src/openvpn/event.c:1139
 knownConditionTrueFalse:src/openvpn/event.c:1148
 # FP: dco_win support has "false" stubs
 knownConditionTrueFalse:src/openvpn/forward.c
 knownConditionTrueFalse:src/openvpn/init.c
 knownConditionTrueFalse:src/openvpn/multi_io.c:163
-# FP: cppcheck thinks that management_query_user_pass is always true,
-# but no idea why
+# 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:1258
+knownConditionTrueFalse:src/openvpn/ssl_openssl.c:1332
 # FP: cppcheck doesn't understand that the function changes szErrMessage
 knownConditionTrueFalse:src/tapctl/main.c:704
 knownConditionTrueFalse:src/openvpnmsica/dllmain.c:164
@@ -50,9 +68,13 @@ 
 missingInclude:src/openvpnserv/common.c:25
 # IGN: strlen(NULL) is not nice code, but seems to work
 nullPointerRedundantCheck:src/openvpn/init.c:299
+# FP: cppcheck doesn't understand ZeroMemory
+redundantAssignment:src/openvpnserv/interactive.c:203
 # IGN: We reuse the same variable name due to macro usage
 shadowVariable:src/openvpn/options.c:2580
 shadowVariable:src/openvpn/options.c:2598
+# 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
 uninitvar:src/openvpn/crypto_epoch.c:60
 # FP: yes, parm is unitialized, but parm_len is 0, so that's handled
@@ -61,6 +83,10 @@ 
 ctuuninitvar:src/openvpn/crypto_mbedtls_legacy.c:698
 uninitvar:src/openvpnserv/interactive.c:1935
 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
 # FP: cppcheck doesn't account for short-circuiting
 unreadVariable:src/openvpn/manage.c:682
 unusedFunction:src/openvpn/siphash_reference.c
diff --git a/dev-tools/openvpn-cppcheck-library.cfg b/dev-tools/openvpn-cppcheck-library.cfg
new file mode 100644
index 0000000..1ea91f4
--- /dev/null
+++ b/dev-tools/openvpn-cppcheck-library.cfg
@@ -0,0 +1,29 @@ 
+<?xml version ="1.0"?>
+<def>
+  <function name="x_msg">
+    <formatstr type="printf"/>
+    <arg nr="2">
+      <formatstr />
+    </arg>
+  </function>
+  <function name="buf_printf">
+    <formatstr type="printf"/>
+    <arg nr="2">
+      <formatstr />
+    </arg>
+  </function>
+  <function name="checked_snprintf">
+    <formatstr type="printf"/>
+    <arg nr="3">
+      <formatstr />
+    </arg>
+  </function>
+  <function name="check_malloc_return">
+    <noreturn>true</noreturn>
+  </function>
+  <define name="HAVE_CONFIG_H" value="1" />
+  <define name="CONFIGURE_SPECIAL_BUILD" value="foo" />
+  <!-- work around macro confusion -->
+  <define name="CMSG_FIRSTHDR" value="cmsg_firsthdr" />
+  <define name="CMSG_NXTHDR" value="cmsg_nxthdr" />
+</def>
diff --git a/dev-tools/run-cppcheck.sh b/dev-tools/run-cppcheck.sh
index 674fc092..40db33e 100755
--- a/dev-tools/run-cppcheck.sh
+++ b/dev-tools/run-cppcheck.sh
@@ -7,20 +7,37 @@ 
 : ${BUILD_DIR:=$PWD}
 : ${INCLUDE_FLAGS:=}
 CPPCHECK_DIR="${BUILD_DIR}/cppcheck_build_dir"
+COMMON_ARGS="-j$(nproc) -q \
+ -DMBEDTLS_SSL_PROTO_TLS1_3 -DMBEDTLS_SSL_KEYING_MATERIAL_EXPORT \
+ -I./include/ -I./tests/unit_tests/openvpn/ \
+ -I./src/compat/ -I./src/openvpn/ -I./src/openvpnserv/ -I./src/plugins/auth-pam/ \
+ -I${BUILD_DIR} -I${BUILD_DIR}/include/ \
+ --enable=all \
+ --library=${SCRIPT_DIR}/openvpn-cppcheck-library.cfg \
+ --library=openssl.cfg \
+ --suppressions-list=${SCRIPT_DIR}/cppcheck-suppression \
+ --cppcheck-build-dir=${CPPCHECK_DIR} \
+ --check-level=exhaustive --max-configs=10 \
+ --error-exitcode=1"
+
 
 set -x
 
 mkdir -p "$CPPCHECK_DIR"
 cd "${SOURCE_DIR}"
-cppcheck -j$(nproc) \
-	 -DHAVE_CONFIG_H -U_WIN32 \
-         -DMBEDTLS_SSL_PROTO_TLS1_3 -DMBEDTLS_SSL_KEYING_MATERIAL_EXPORT \
-	 -I./include/ -I./tests/unit_tests/openvpn/ \
-	 -I./src/compat/ -I./src/openvpn/ -I./src/openvpnserv/ -I./src/plugins/auth-pam/ \
-	 -I"${BUILD_DIR}" -I"${BUILD_DIR}/include/" $INCLUDE_FLAGS \
-	 --enable=all \
-	 --suppressions-list="${SCRIPT_DIR}/cppcheck-suppression" \
-	 --cppcheck-build-dir="${CPPCHECK_DIR}" \
-	 --check-level=exhaustive \
-	 --error-exitcode=1 \
-	 src/ tests/ sample/
+cppcheck $COMMON_ARGS $INCLUDE_FLAGS \
+         --platform=unix64 \
+         --library=posix.cfg --library=bsd.cfg --library=gnu.cfg \
+         -U_WIN32 \
+         src/openvpn/ src/compat/ src/plugins/ sample/ \
+         tests/unit_tests/example_test/ tests/unit_tests/openvpn/ \
+         tests/unit_tests/plugins/
+cppcheck $COMMON_ARGS \
+         --platform=win64 \
+         --library=windows.cfg \
+         -D_WIN32 \
+         -UTARGET_LINUX -UTARGET_FREEBSD -UTARGET_OPENBSD -UTARGET_NETBSD \
+         -UTARGET_DARWIN -UTARGET_ANDROID -UTARGET_SOLARIS -UTARGET_DRAGONFLY \
+         -UTARGET_AIX \
+         src/openvpn* src/compat/ \
+         tests/unit_tests/example_test/ tests/unit_tests/openvpn*