I updated to the latest tree and found I was getting some new cppcheck false positives. I suspect these are fixed in the latest upstream cppcheck, but they're still there for the one packaged in Fedora 37, which is a lot more convenient for me to use. Here are some workarounds for those failures. David Gibson (2): Work around weird false positives with cppcheck-2.9.1 Fix false positive if cppcheck doesn't give a false positive conf.c | 2 +- netlink.c | 2 +- tap.c | 2 +- tcp.c | 4 ++-- tcp_splice.c | 2 +- udp.c | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) -- 2.39.2
Commit 89e38f55 "treewide: Fix header includes to build with musl" added extra #includes to work with musl. Unfortunately with the cppcheck version I'm using (cppcheck-2.9-1.fc37.x86_64 in Fedora 37) this causes weird false positives: specifically cppcheck seems to hit a #error in <bits/unistd.h> complaining about including it directly instead of via <unistd.h> (which is not something we're doing). I have no idea why that would be happening; but I'm guessing it has to be a bug in the cpp implementation in that cppcheck version. In any case, it's possible to work around this by moving the include of <unistd.h> before the include of <signal.h>. So, do that. Fixes: 89e38f55 Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- conf.c | 2 +- netlink.c | 2 +- tap.c | 2 +- tcp.c | 2 +- tcp_splice.c | 2 +- udp.c | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/conf.c b/conf.c index ce60432..04e9956 100644 --- a/conf.c +++ b/conf.c @@ -23,12 +23,12 @@ #include <limits.h> #include <grp.h> #include <pwd.h> +#include <unistd.h> #include <signal.h> #include <stdlib.h> #include <stdint.h> #include <stdio.h> #include <stdbool.h> -#include <unistd.h> #include <syslog.h> #include <time.h> #include <netinet/in.h> diff --git a/netlink.c b/netlink.c index c8d39a1..00aa3e9 100644 --- a/netlink.c +++ b/netlink.c @@ -18,11 +18,11 @@ #include <errno.h> #include <sys/types.h> #include <limits.h> +#include <unistd.h> #include <signal.h> #include <stdlib.h> #include <stdbool.h> #include <stdint.h> -#include <unistd.h> #include <arpa/inet.h> #include <netinet/in.h> #include <netinet/if_ether.h> diff --git a/tap.c b/tap.c index 15fb52e..68ef480 100644 --- a/tap.c +++ b/tap.c @@ -14,6 +14,7 @@ */ #include <sched.h> +#include <unistd.h> #include <signal.h> #include <stdio.h> #include <errno.h> @@ -33,7 +34,6 @@ #include <sys/uio.h> #include <stdbool.h> #include <stdlib.h> -#include <unistd.h> #include <netinet/ip.h> #include <netinet/tcp.h> #include <netinet/udp.h> diff --git a/tcp.c b/tcp.c index 0214087..8551416 100644 --- a/tcp.c +++ b/tcp.c @@ -267,6 +267,7 @@ #include <sched.h> #include <fcntl.h> #include <stdio.h> +#include <unistd.h> #include <signal.h> #include <stdlib.h> #include <errno.h> @@ -287,7 +288,6 @@ #include <sys/timerfd.h> #include <sys/types.h> #include <sys/uio.h> -#include <unistd.h> #include <time.h> #include <linux/tcp.h> /* For struct tcp_info */ diff --git a/tcp_splice.c b/tcp_splice.c index 6559762..5bfad2a 100644 --- a/tcp_splice.c +++ b/tcp_splice.c @@ -32,6 +32,7 @@ */ #include <sched.h> +#include <unistd.h> #include <signal.h> #include <errno.h> #include <fcntl.h> @@ -40,7 +41,6 @@ #include <stdbool.h> #include <string.h> #include <time.h> -#include <unistd.h> #include <net/ethernet.h> #include <netinet/in.h> #include <netinet/tcp.h> diff --git a/udp.c b/udp.c index b7bc4f3..70732fc 100644 --- a/udp.c +++ b/udp.c @@ -91,6 +91,7 @@ */ #include <sched.h> +#include <unistd.h> #include <signal.h> #include <stdio.h> #include <errno.h> @@ -107,7 +108,6 @@ #include <sys/types.h> #include <sys/socket.h> #include <sys/uio.h> -#include <unistd.h> #include <time.h> #include "checksum.h" -- 2.39.2
On Tue, 21 Mar 2023 14:54:59 +1100 David Gibson <david(a)gibson.dropbear.id.au> wrote:Commit 89e38f55 "treewide: Fix header includes to build with musl" added extra #includes to work with musl. Unfortunately with the cppcheck version I'm using (cppcheck-2.9-1.fc37.x86_64 in Fedora 37) this causes weird false positives: specifically cppcheck seems to hit a #error in <bits/unistd.h> complaining about including it directly instead of via <unistd.h> (which is not something we're doing). I have no idea why that would be happening; but I'm guessing it has to be a bug in the cpp implementation in that cppcheck version. In any case, it's possible to work around this by moving the include of <unistd.h> before the include of <signal.h>. So, do that.Checked on Alpine, build against musl still works with this. Applied, I'll push it out in a bit. -- Stefano
da46fdac "tcp: Suppress knownConditionTrueFalse cppcheck false positive" introduced a suppression to work around a cppcheck bug causing a false positive warning. However, the suppression will itself cause a spurious unmatchedSuppression warning if used with a version of cppcheck from before the bug was introduced. That includes the packaged version of cppcheck in Fedora. Suppress the unmatchedSuppression as well. Fixes: da46fdac Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- tcp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tcp.c b/tcp.c index 8551416..d5e6607 100644 --- a/tcp.c +++ b/tcp.c @@ -1822,7 +1822,7 @@ static void tcp_clamp_window(const struct ctx *c, struct tcp_tap_conn *conn, * * drop this suppression once that's resolved. */ - /* cppcheck-suppress knownConditionTrueFalse */ + /* cppcheck-suppress [knownConditionTrueFalse, unmatchedSuppression] */ if ((wnd > prev_scaled && wnd * 99 / 100 < prev_scaled) || (wnd < prev_scaled && wnd * 101 / 100 > prev_scaled)) return; -- 2.39.2
On Tue, 21 Mar 2023 14:55:00 +1100 David Gibson <david(a)gibson.dropbear.id.au> wrote:da46fdac "tcp: Suppress knownConditionTrueFalse cppcheck false positive" introduced a suppression to work around a cppcheck bug causing a false positive warning. However, the suppression will itself cause a spurious unmatchedSuppression warning if used with a version of cppcheck from before the bug was introduced. That includes the packaged version of cppcheck in Fedora. Suppress the unmatchedSuppression as well.Oops, I always forget about those. Applied. -- Stefano