[PATCH 0/6] Deal with a bunch of staticFunction warnings from cppcheck 2.17

newer
[PATCH v4 0/1] Improve validation...

David Gibson

5 Mar 2025 5 Mar '25

7:15 a.m.

The new cppcheck version in Fedora 41 is a bunch more aggressive with staticFunction warnings, about exposed functions that aren't used outside their .c file. Deal with these warnings. David Gibson (6): treewide: Mark assorted functions static log: Don't export passt_vsyslog() checksum: Don't export various functions tcp: Don't export tcp_update_csum() vhost_user: Don't export several functions cppcheck: Add suppressions for "logically" exported functions checksum.c | 34 ++++++++++++++++----------------- checksum.h | 3 --- iov.c | 1 + log.c | 51 +++++++++++++++++++++++++------------------------- log.h | 1 - netlink.c | 2 +- passt.c | 2 +- tcp.c | 9 +++++---- tcp_internal.h | 2 -- vhost_user.c | 2 +- vhost_user.h | 1 - virtio.c | 9 +++++---- virtio.h | 4 ---- 13 files changed, 57 insertions(+), 64 deletions(-) -- 2.48.1

Show replies by date

David Gibson

5 Mar 5 Mar

7:15 a.m.

New subject: [PATCH 1/6] treewide: Mark assorted functions static

From: David Gibson This marks static a number of functions which are only used in their .c file, have no prototypes in a .h and were never intended to be globally exposed. Signed-off-by: David Gibson --- log.c | 2 +- netlink.c | 2 +- passt.c | 2 +- tcp.c | 6 +++--- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/log.c b/log.c index 95e45762..b6bce214 100644 --- a/log.c +++ b/log.c @@ -56,7 +56,7 @@ bool log_stderr = true; /* Not daemonised, no shell spawned */ * * Return: pointer to @now, or NULL if there was an error retrieving the time */ -const struct timespec *logtime(struct timespec *ts) +static const struct timespec *logtime(struct timespec *ts) { if (clock_gettime(CLOCK_MONOTONIC, ts)) return NULL; diff --git a/netlink.c b/netlink.c index 37d8b5bc..a0525047 100644 --- a/netlink.c +++ b/netlink.c @@ -355,7 +355,7 @@ unsigned int nl_get_ext_if(int s, sa_family_t af) * * Return: true if a gateway was found, false otherwise */ -bool nl_route_get_def_multipath(struct rtattr *rta, void *gw) +static bool nl_route_get_def_multipath(struct rtattr *rta, void *gw) { int nh_len = RTA_PAYLOAD(rta); struct rtnexthop *rtnh; diff --git a/passt.c b/passt.c index 68d1a283..868842b0 100644 --- a/passt.c +++ b/passt.c @@ -166,7 +166,7 @@ void proto_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s) * * #syscalls exit_group */ -void exit_handler(int signal) +static void exit_handler(int signal) { (void)signal; diff --git a/tcp.c b/tcp.c index b3aa9a2c..1db16095 100644 --- a/tcp.c +++ b/tcp.c @@ -2427,7 +2427,7 @@ static void tcp_ns_sock_init6(const struct ctx *c, in_port_t port) * @c: Execution context * @port: Port, host order */ -void tcp_ns_sock_init(const struct ctx *c, in_port_t port) +static void tcp_ns_sock_init(const struct ctx *c, in_port_t port) { ASSERT(!c->no_tcp); @@ -3071,7 +3071,7 @@ static int tcp_flow_dump_rcvqueue(int s, struct tcp_tap_transfer_ext *t) * * Return: 0 on success, negative error code on failure */ -int tcp_flow_repair_opt(int s, const struct tcp_tap_transfer_ext *t) +static int tcp_flow_repair_opt(int s, const struct tcp_tap_transfer_ext *t) { const struct tcp_repair_opt opts[] = { { TCPOPT_WINDOW, t->snd_ws + (t->rcv_ws << 16) }, @@ -3263,7 +3263,7 @@ fail: * * Return: 0 on success, negative error code on failure */ -int tcp_flow_repair_socket(struct ctx *c, struct tcp_tap_conn *conn) +static int tcp_flow_repair_socket(struct ctx *c, struct tcp_tap_conn *conn) { sa_family_t af = CONN_V4(conn) ? AF_INET : AF_INET6; const struct flowside *sockside = HOSTFLOW(conn); -- 2.48.1

David Gibson

7:15 a.m.

New subject: [PATCH 2/6] log: Don't export passt_vsyslog()

passt_vsyslog() is an exposed function in log.h. However it shouldn't be called from outside log.c: it writes specifically to the system log, and most code should call passt's logging helpers which might go to the syslog or to a log file. Make passt_vsyslog() local to log.c. This requires a code motion to avoid a forward declaration. Signed-off-by: David Gibson --- log.c | 48 ++++++++++++++++++++++++------------------------ log.h | 1 - 2 files changed, 24 insertions(+), 25 deletions(-) diff --git a/log.c b/log.c index b6bce214..6eda4c4c 100644 --- a/log.c +++ b/log.c @@ -249,6 +249,30 @@ static void logfile_write(bool newline, bool cont, int pri, log_written += n; } +/** + * passt_vsyslog() - vsyslog() implementation not using heap memory + * @newline: Append newline at the end of the message, if missing + * @pri: Facility and level map, same as priority for vsyslog() + * @format: Same as vsyslog() format + * @ap: Same as vsyslog() ap + */ +static void passt_vsyslog(bool newline, int pri, const char *format, va_list ap) +{ + char buf[BUFSIZ]; + int n; + + /* Send without timestamp, the system logger should add it */ + n = snprintf(buf, BUFSIZ, "<%i> %s: ", pri, log_ident); + + n += vsnprintf(buf + n, BUFSIZ - n, format, ap); + + if (newline && format[strlen(format)] != '\n') + n += snprintf(buf + n, BUFSIZ - n, "\n"); + + if (log_sock >= 0 && send(log_sock, buf, n, 0) != n && log_stderr) + FPRINTF(stderr, "Failed to send %i bytes to syslog\n", n); +} + /** * vlogmsg() - Print or send messages to log or output files as configured * @newline: Append newline at the end of the message, if missing @@ -373,30 +397,6 @@ void __setlogmask(int mask) setlogmask(mask); } -/** - * passt_vsyslog() - vsyslog() implementation not using heap memory - * @newline: Append newline at the end of the message, if missing - * @pri: Facility and level map, same as priority for vsyslog() - * @format: Same as vsyslog() format - * @ap: Same as vsyslog() ap - */ -void passt_vsyslog(bool newline, int pri, const char *format, va_list ap) -{ - char buf[BUFSIZ]; - int n; - - /* Send without timestamp, the system logger should add it */ - n = snprintf(buf, BUFSIZ, "<%i> %s: ", pri, log_ident); - - n += vsnprintf(buf + n, BUFSIZ - n, format, ap); - - if (newline && format[strlen(format)] != '\n') - n += snprintf(buf + n, BUFSIZ - n, "\n"); - - if (log_sock >= 0 && send(log_sock, buf, n, 0) != n && log_stderr) - FPRINTF(stderr, "Failed to send %i bytes to syslog\n", n); -} - /** * logfile_init() - Open log file and write header with PID, version, path * @name: Identifier for header: passt or pasta diff --git a/log.h b/log.h index 22c7b9ab..08aa88c1 100644 --- a/log.h +++ b/log.h @@ -55,7 +55,6 @@ void trace_init(int enable); void __openlog(const char *ident, int option, int facility); void logfile_init(const char *name, const char *path, size_t size); -void passt_vsyslog(bool newline, int pri, const char *format, va_list ap); void __setlogmask(int mask); #endif /* LOG_H */ -- 2.48.1

David Gibson

7:15 a.m.

New subject: [PATCH 3/6] checksum: Don't export various functions

Several of the exposed functions in checksum.h are no longer directly used. Remove them from the header, and make static. In particular sum_16b() should not be used outside: generally csum_unfolded() should be used which will automatically use either the AVX2 optimized version or sum_16b() as necessary. csum_fold() and csum() could have external uses, but they're not used right now. We can expose them again if we need to. Signed-off-by: David Gibson --- checksum.c | 34 +++++++++++++++++----------------- checksum.h | 3 --- 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/checksum.c b/checksum.c index b01e0fe7..0894eca8 100644 --- a/checksum.c +++ b/checksum.c @@ -85,7 +85,7 @@ */ /* NOLINTNEXTLINE(clang-diagnostic-unknown-attributes) */ __attribute__((optimize("-fno-strict-aliasing"))) -uint32_t sum_16b(const void *buf, size_t len) +static uint32_t sum_16b(const void *buf, size_t len) { const uint16_t *p = buf; uint32_t sum = 0; @@ -107,7 +107,7 @@ uint32_t sum_16b(const void *buf, size_t len) * * Return: 16-bit folded sum */ -uint16_t csum_fold(uint32_t sum) +static uint16_t csum_fold(uint32_t sum) { while (sum >> 16) sum = (sum & 0xffff) + (sum >> 16); @@ -161,6 +161,21 @@ uint32_t proto_ipv4_header_psum(uint16_t l4len, uint8_t protocol, return psum; } +/** + * csum() - Compute TCP/IP-style checksum + * @buf: Input buffer + * @len: Input length + * @init: Initial 32-bit checksum, 0 for no pre-computed checksum + * + * Return: 16-bit folded, complemented checksum + */ +/* NOLINTNEXTLINE(clang-diagnostic-unknown-attributes) */ +__attribute__((optimize("-fno-strict-aliasing"))) /* See csum_16b() */ +static uint16_t csum(const void *buf, size_t len, uint32_t init) +{ + return (uint16_t)~csum_fold(csum_unfolded(buf, len, init)); +} + /** * csum_udp4() - Calculate and set checksum for a UDP over IPv4 packet * @udp4hr: UDP header, initialised apart from checksum @@ -482,21 +497,6 @@ uint32_t csum_unfolded(const void *buf, size_t len, uint32_t init) } #endif /* !__AVX2__ */ -/** - * csum() - Compute TCP/IP-style checksum - * @buf: Input buffer - * @len: Input length - * @init: Initial 32-bit checksum, 0 for no pre-computed checksum - * - * Return: 16-bit folded, complemented checksum - */ -/* NOLINTNEXTLINE(clang-diagnostic-unknown-attributes) */ -__attribute__((optimize("-fno-strict-aliasing"))) /* See csum_16b() */ -uint16_t csum(const void *buf, size_t len, uint32_t init) -{ - return (uint16_t)~csum_fold(csum_unfolded(buf, len, init)); -} - /** * csum_iov_tail() - Calculate unfolded checksum for the tail of an IO vector * @tail: IO vector tail to checksum diff --git a/checksum.h b/checksum.h index e243c971..683a09bc 100644 --- a/checksum.h +++ b/checksum.h @@ -11,8 +11,6 @@ struct icmphdr; struct icmp6hdr; struct iov_tail; -uint32_t sum_16b(const void *buf, size_t len); -uint16_t csum_fold(uint32_t sum); uint16_t csum_unaligned(const void *buf, size_t len, uint32_t init); uint16_t csum_ip4_header(uint16_t l3len, uint8_t protocol, struct in_addr saddr, struct in_addr daddr); @@ -32,7 +30,6 @@ void csum_icmp6(struct icmp6hdr *icmp6hr, const struct in6_addr *saddr, const struct in6_addr *daddr, const void *payload, size_t dlen); uint32_t csum_unfolded(const void *buf, size_t len, uint32_t init); -uint16_t csum(const void *buf, size_t len, uint32_t init); uint16_t csum_iov_tail(struct iov_tail *tail, uint32_t init); #endif /* CHECKSUM_H */ -- 2.48.1

David Gibson

7:15 a.m.

New subject: [PATCH 4/6] tcp: Don't export tcp_update_csum()

tcp_update_csum() is exposed in tcp_internal.h, but is only used in tcp.c. Remove the unneded prototype and make it static. Signed-off-by: David Gibson --- tcp.c | 3 ++- tcp_internal.h | 2 -- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/tcp.c b/tcp.c index 1db16095..b8a01cc6 100644 --- a/tcp.c +++ b/tcp.c @@ -787,7 +787,8 @@ static void tcp_sock_set_nodelay(int s) * @th: TCP header (updated) * @payload: TCP payload */ -void tcp_update_csum(uint32_t psum, struct tcphdr *th, struct iov_tail *payload) +static void tcp_update_csum(uint32_t psum, struct tcphdr *th, + struct iov_tail *payload) { th->check = 0; psum = csum_unfolded(th, sizeof(*th), psum); diff --git a/tcp_internal.h b/tcp_internal.h index 9cf31f53..6f5e054c 100644 --- a/tcp_internal.h +++ b/tcp_internal.h @@ -166,8 +166,6 @@ void tcp_rst_do(const struct ctx *c, struct tcp_tap_conn *conn); struct tcp_info_linux; -void tcp_update_csum(uint32_t psum, struct tcphdr *th, - struct iov_tail *payload); void tcp_fill_headers(const struct tcp_tap_conn *conn, struct tap_hdr *taph, struct iphdr *ip4h, struct ipv6hdr *ip6h, -- 2.48.1

David Gibson

7:15 a.m.

New subject: [PATCH 5/6] vhost_user: Don't export several functions

vhost-user added several functions which are exposed in headers, but not used outside the file where they're defined. I can't tell if these are really internal functions, or of they're logically supposed to be exported, but we don't happen to have anything using them yet. For the time being, just remove the exports. We can add them back if we need to. Signed-off-by: David Gibson --- vhost_user.c | 2 +- vhost_user.h | 1 - virtio.c | 9 +++++---- virtio.h | 4 ---- 4 files changed, 6 insertions(+), 10 deletions(-) diff --git a/vhost_user.c b/vhost_user.c index be1aa942..105f77a5 100644 --- a/vhost_user.c +++ b/vhost_user.c @@ -517,7 +517,7 @@ static void vu_close_log(struct vu_dev *vdev) * vu_log_kick() - Inform the front-end that the log has been modified * @vdev: vhost-user device */ -void vu_log_kick(const struct vu_dev *vdev) +static void vu_log_kick(const struct vu_dev *vdev) { if (vdev->log_call_fd != -1) { int rc; diff --git a/vhost_user.h b/vhost_user.h index e769cb14..1daacd1e 100644 --- a/vhost_user.h +++ b/vhost_user.h @@ -241,7 +241,6 @@ static inline bool vu_queue_started(const struct vu_virtq *vq) void vu_print_capabilities(void); void vu_init(struct ctx *c); void vu_cleanup(struct vu_dev *vdev); -void vu_log_kick(const struct vu_dev *vdev); void vu_log_write(const struct vu_dev *vdev, uint64_t address, uint64_t length); void vu_control_handler(struct vu_dev *vdev, int fd, uint32_t events); diff --git a/virtio.c b/virtio.c index 2b58e4df..bc2b89a7 100644 --- a/virtio.c +++ b/virtio.c @@ -286,7 +286,7 @@ static int virtqueue_read_next_desc(const struct vring_desc *desc, * * Return: true if the virtqueue is empty, false otherwise */ -bool vu_queue_empty(struct vu_virtq *vq) +static bool vu_queue_empty(struct vu_virtq *vq) { if (!vq->vring.avail) return true; @@ -671,9 +671,10 @@ static void vu_log_queue_fill(const struct vu_dev *vdev, struct vu_virtq *vq, * @len: Size of the element * @idx: Used ring entry index */ -void vu_queue_fill_by_index(const struct vu_dev *vdev, struct vu_virtq *vq, - unsigned int index, unsigned int len, - unsigned int idx) +static void vu_queue_fill_by_index(const struct vu_dev *vdev, + struct vu_virtq *vq, + unsigned int index, unsigned int len, + unsigned int idx) { struct vring_used_elem uelem; diff --git a/virtio.h b/virtio.h index 0a59441b..7a370bde 100644 --- a/virtio.h +++ b/virtio.h @@ -174,16 +174,12 @@ static inline bool vu_has_protocol_feature(const struct vu_dev *vdev, return has_feature(vdev->protocol_features, fbit); } -bool vu_queue_empty(struct vu_virtq *vq); void vu_queue_notify(const struct vu_dev *dev, struct vu_virtq *vq); int vu_queue_pop(const struct vu_dev *dev, struct vu_virtq *vq, struct vu_virtq_element *elem); void vu_queue_detach_element(struct vu_virtq *vq); void vu_queue_unpop(struct vu_virtq *vq); bool vu_queue_rewind(struct vu_virtq *vq, unsigned int num); -void vu_queue_fill_by_index(const struct vu_dev *vdev, struct vu_virtq *vq, - unsigned int index, unsigned int len, - unsigned int idx); void vu_queue_fill(const struct vu_dev *vdev, struct vu_virtq *vq, const struct vu_virtq_element *elem, unsigned int len, unsigned int idx); -- 2.48.1

David Gibson

7:15 a.m.

New subject: [PATCH 6/6] cppcheck: Add suppressions for "logically" exported functions

We have some functions in our headers which are definitely there on purpose. However, they're not yet used outside the files in which they're defined. That causes sufficiently recent cppcheck versions (2.17) to complain they should be static. Suppress the errors for these "logically" exported functions. Signed-off-by: David Gibson --- iov.c | 1 + log.c | 1 + 2 files changed, 2 insertions(+) diff --git a/iov.c b/iov.c index 3b122726..8c63b7ea 100644 --- a/iov.c +++ b/iov.c @@ -203,6 +203,7 @@ size_t iov_tail_size(struct iov_tail *tail) * overruns the IO vector, is not contiguous or doesn't have the * requested alignment. */ +/* cppcheck-suppress [staticFunction,unmatchedSuppression] */ void *iov_peek_header_(struct iov_tail *tail, size_t len, size_t align) { char *p; diff --git a/log.c b/log.c index 6eda4c4c..d40d7ae2 100644 --- a/log.c +++ b/log.c @@ -281,6 +281,7 @@ static void passt_vsyslog(bool newline, int pri, const char *format, va_list ap) * @format: Message * @ap: Variable argument list */ +/* cppcheck-suppress [staticFunction,unmatchedSuppression] */ void vlogmsg(bool newline, bool cont, int pri, const char *format, va_list ap) { bool debug_print = (log_mask & LOG_MASK(LOG_DEBUG)) && log_file == -1; -- 2.48.1

Stefano Brivio

7 Mar 7 Mar

7:30 a.m.

On Wed, 5 Mar 2025 17:15:02 +1100 David Gibson wrote:

...

The new cppcheck version in Fedora 41 is a bunch more aggressive with staticFunction warnings, about exposed functions that aren't used outside their .c file. Deal with these warnings.

David Gibson (6): treewide: Mark assorted functions static log: Don't export passt_vsyslog() checksum: Don't export various functions tcp: Don't export tcp_update_csum() vhost_user: Don't export several functions cppcheck: Add suppressions for "logically" exported functions

Applied. -- Stefano

Age (days ago)

Last active (days ago)

List overview

Download

7 comments

2 participants

participants (2)

David Gibson
Stefano Brivio

[PATCH 0/6] Deal with a bunch of staticFunction warnings from cppcheck 2.17

tags

participants (2)