We pass -I options to cppcheck so that it will find the system headers. Then we need to pass a bunch more options to suppress the zillions of cppcheck errors found in those headers. It turns out, however, that it's not recommended to give the system headers to cppcheck anyway. Instead it has built-in knowledge of the ANSI libc and uses that as the basis of its checks. We do need to suppress missingIncludeSystem warnings instead though. Not bothering with the system headers makes the cppcheck runtime go from ~37s to ~14s on my machine, which is a pretty nice win. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
log.c has several #ifdefs on FALLOC_FL_COLLAPSE_RANGE that won't attempt to use it if not defined. But even if the value is defined at compile time, it might not be available in the runtime kernel, so we need to check for errors from a fallocate() call and fall back to other methods. Simplify this to only need the runtime check by using linux_dep.h to define FALLOC_FL_COLLAPSE_RANGE if it's not in the kernel headers. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
While experimenting with cppcheck options, I hit several false positives caused by this bug: https://trac.cppcheck.net/ticket/13227 Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
We probe the available stack limit in the Makefile using rlimit, then use that to set the size of the stack when we clone() extra threads. But the rlimit at compile time need not be the same as the rlimit at runtime, so that's not particularly sensible. Ideally, we'd set the stack size based on an estimate of the actual maximum stack usage of all our clone()ed functions. We don't have that at the moment, but to keep things simple just set it to 1MiB - that's what the current probe will set things to on my default configuration Fedora 40, so it's likely to be fine in most cases. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
We insert -DARCH for all compiles, based on TARGET_ARCH determined in the Makefile. However, this is only used in qrap.c, not anywhere else in passt or pasta. Only supply this -D when compiling qrap specifically. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Currently we construct the AUDIT_ARCH variable in the Makefile, then pass it into the C code with -D. The only place that uses it, though is the BPF filter generated by seccomp.sh. seccomp.sh already needs to do things differently depending on the arch, so it might as well just insert the expanded AUDIT_ARCH directly into the generated code, rather than using a #define. Arguably this is better, even, since it ensures more locally that the arch the BPF checks for matches the arch seccomp.sh built the filter for. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
NETNS_RUN_DIR is set in the Makefile, then passed into the C code with -D. But NETNS_RUN_DIR is just a fixed string, it doesn't depend on any make probes or variables, so there's really no reason to handle it via the Makefile. Just move it to a plain #define in conf.c. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Currently we configure clang-tidy with a very long command line spelled out in the Makefile (mostly a big list of lints to disable). Move it from here into a .clang-tidy configuration file, so that the config is accessible if clang-tidy is invoked in other ways (e.g. via clangd) as well. As a bonus this also means that we can move the bulky comments about why we're suppressing various tests inline with the relevant config lines. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
There are things in qrap.c that clang-tidy complains about that aren't worth fixing. So, we currently exclude it using $(filter-out). However, we already have a make variable which has just the passt sources, excluding qrap, so we can use that instead of the awkward filter-out expression. Currently, we still include qrap.c for cppcheck, but there's not much point doing so: it's, well, qrap, so we don't care that much about lints. Exclude it from cppcheck as well, for consistency. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
With clang-tidy and LLVM 19:
/home/sbrivio/passt/conf.c:1218:29: error: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses,-warnings-as-errors]
1218 | const char *octet = str + 3 * i;
| ^~~~~~
| ( )
/home/sbrivio/passt/ndp.c:285:18: error: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses,-warnings-as-errors]
285 | .len = 1 + 2 * n,
| ^~~~~~
| ( )
/home/sbrivio/passt/ndp.c:329:23: error: '%' has higher precedence than '-'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses,-warnings-as-errors]
329 | memset(ptr, 0, 8 - dns_s_len % 8); /* padding */
| ^~~~~~~~~~~~~~
| ( )
/home/sbrivio/passt/pcap.c:131:20: error: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses,-warnings-as-errors]
131 | pcap_frame(iov + i * frame_parts, frame_parts, offset, &now);
| ^~~~~~~~~~~~~~~~
| ( )
/home/sbrivio/passt/util.c:216:10: error: '/' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses,-warnings-as-errors]
216 | return (a->tv_nsec + 1000000000 - b->tv_nsec) / 1000 +
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| ( )
/home/sbrivio/passt/util.c:217:10: error: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses,-warnings-as-errors]
217 | (a->tv_sec - b->tv_sec - 1) * 1000000;
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| ( )
/home/sbrivio/passt/util.c:220:9: error: '/' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses,-warnings-as-errors]
220 | return (a->tv_nsec - b->tv_nsec) / 1000 +
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| ( )
/home/sbrivio/passt/util.c:221:9: error: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses,-warnings-as-errors]
221 | (a->tv_sec - b->tv_sec) * 1000000;
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| ( )
/home/sbrivio/passt/util.c:545:32: error: '/' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses,-warnings-as-errors]
545 | return clone(fn, stack_area + stack_size / 2, flags, arg);
| ^~~~~~~~~~~~~~~
| ( )
Just... no.
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>