[PATCH 0/2] Fix some problems with cppcheck-2.9.1 (from Fedora 37)

[PATCH 0/2] Fix some problems with cppcheck-2.9.1 (from Fedora 37)

[David Gibson]

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

[PATCH 1/2] Work around weird false positives with cppcheck-2.9.1

[David Gibson]

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@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"
-- 
@@ -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

[PATCH 2/2] Fix false positive if cppcheck doesn't give a false positive

[David Gibson]

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@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;
-- 
@@ -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

Re: [PATCH 1/2] Work around weird false positives with cppcheck-2.9.1

[Stefano Brivio]

On Tue, 21 Mar 2023 14:54:59 +1100
David Gibson <david@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

Re: [PATCH 2/2] Fix false positive if cppcheck doesn't give a false positive

[Stefano Brivio]

On Tue, 21 Mar 2023 14:55:00 +1100
David Gibson <david@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