From: Frank Lichtenheld <[email protected]> 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 <[email protected]> Acked-by: Gert Doering <[email protected]> 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 <[email protected]> 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* _______________________________________________ Openvpn-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openvpn-devel
