[PATCH 0/3] Assorted fixes

[PATCH 0/3] Assorted fixes

[David Gibson]

Came back from holidays to find a few things broken by external
changes.  Here are the fixes for them.

David Gibson (3):
  test: Fix passt.mbuto for cases where /usr/sbin doesn't exist
  treewide: Make a bunch of pointer variables pointers to const
  pif: Remove unused pif_name() function

 arch.c           |  3 ++-
 conf.c           | 12 +++++++-----
 dhcp.c           |  9 +++++----
 dhcpv6.c         |  9 +++++----
 log.c            |  6 ++++--
 pif.h            |  1 +
 qrap.c           |  4 ++--
 tap.c            | 12 ++++++------
 tcp.c            |  8 ++++----
 tcp_splice.c     |  2 +-
 test/passt.mbuto |  2 +-
 udp.c            |  6 +++---
 util.c           |  4 ++--
 13 files changed, 43 insertions(+), 35 deletions(-)

-- 
2.43.0

[PATCH 1/3] test: Fix passt.mbuto for cases where /usr/sbin doesn't exist

[David Gibson]

f0ccca74 ("test: make passt.mbuto script more robust") is supposed to make
mbuto more robust by standardizing on always putting things in /usr/sbin
with /sbin a symlink to it.  This matters because different distros have
different conventions about how the two are used.

However, the logic there requires that /usr/sbin at least exists to start
with.  This isn't always the case with Fedora derived mbuto images.
Ironically the DIRS variable ensures that /sbin exists, although we then
remove it, but doesn't require /usr/sbin to exist.  Fix that up so that
the new logic will work with Fedora.

Fixes: f0ccca74 ("test: make passt.mbuto script more robust")
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 test/passt.mbuto | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/passt.mbuto b/test/passt.mbuto
index 75d1260..6240d5c 100755
--- a/test/passt.mbuto
+++ b/test/passt.mbuto
@@ -21,7 +21,7 @@ LINKS="${LINKS:-
 	 ash,dash,bash		/init
 	 ash,dash,bash		/bin/sh}"
 
-DIRS="${DIRS} /tmp /sbin /usr/share /var/log /var/lib /etc/ssh /run/sshd /root/.ssh"
+DIRS="${DIRS} /tmp /usr/sbin /usr/share /var/log /var/lib /etc/ssh /run/sshd /root/.ssh"
 
 COPIES="${COPIES} small.bin,/root/small.bin medium.bin,/root/medium.bin big.bin,/root/big.bin"
 
-- 
@@ -21,7 +21,7 @@ LINKS="${LINKS:-
 	 ash,dash,bash		/init
 	 ash,dash,bash		/bin/sh}"
 
-DIRS="${DIRS} /tmp /sbin /usr/share /var/log /var/lib /etc/ssh /run/sshd /root/.ssh"
+DIRS="${DIRS} /tmp /usr/sbin /usr/share /var/log /var/lib /etc/ssh /run/sshd /root/.ssh"
 
 COPIES="${COPIES} small.bin,/root/small.bin medium.bin,/root/medium.bin big.bin,/root/big.bin"
 
-- 
2.43.0

[PATCH 2/3] treewide: Make a bunch of pointer variables pointers to const

[David Gibson]

Sufficiently recent cppcheck (I'm using 2.13.0) seems to have added another
warning for pointer variables which could be pointer to const but aren't.
Use this to make a bunch of variables const pointers where they previously
weren't for no particular reason.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 arch.c       |  3 ++-
 conf.c       | 12 +++++++-----
 dhcp.c       |  9 +++++----
 dhcpv6.c     |  9 +++++----
 log.c        |  6 ++++--
 qrap.c       |  4 ++--
 tap.c        | 12 ++++++------
 tcp.c        |  8 ++++----
 tcp_splice.c |  2 +-
 udp.c        |  6 +++---
 util.c       |  4 ++--
 11 files changed, 41 insertions(+), 34 deletions(-)

diff --git a/arch.c b/arch.c
index 012123c..80a41bc 100644
--- a/arch.c
+++ b/arch.c
@@ -25,7 +25,8 @@
 #ifdef __x86_64__
 void arch_avx2_exec(char **argv)
 {
-	char exe[PATH_MAX] = { 0 }, *p;
+	char exe[PATH_MAX] = { 0 };
+	const char *p;
 
 	if (readlink("/proc/self/exe", exe, PATH_MAX - 1) < 0) {
 		perror("readlink /proc/self/exe");
diff --git a/conf.c b/conf.c
index ad2a093..5e15b66 100644
--- a/conf.c
+++ b/conf.c
@@ -412,8 +412,9 @@ static void get_dns(struct ctx *c)
 	int dns4_set, dns6_set, dnss_set, dns_set, fd;
 	struct fqdn *s = c->dns_search;
 	struct lineread resolvconf;
+	char *line, *end;
+	const char *p;
 	int line_len;
-	char *line, *p, *end;
 
 	dns4_set = !c->ifi4 || !IN4_IS_ADDR_UNSPECIFIED(dns4);
 	dns6_set = !c->ifi6 || !IN6_IS_ADDR_UNSPECIFIED(dns6);
@@ -1025,7 +1026,7 @@ static int conf_runas(char *opt, unsigned int *uid, unsigned int *gid)
 	if (*endptr) {
 #ifndef GLIBC_NO_STATIC_NSS
 		/* Not numeric, look up as a username */
-		struct passwd *pw;
+		const struct passwd *pw;
 		/* cppcheck-suppress getpwnamCalled */
 		if (!(pw = getpwnam(uopt)) || !(*uid = pw->pw_uid))
 			return -ENOENT;
@@ -1042,7 +1043,7 @@ static int conf_runas(char *opt, unsigned int *uid, unsigned int *gid)
 	if (*endptr) {
 #ifndef GLIBC_NO_STATIC_NSS
 		/* Not numeric, look up as a group name */
-		struct group *gr;
+		const struct group *gr;
 		/* cppcheck-suppress getgrnamCalled */
 		if (!(gr = getgrnam(gopt)))
 			return -ENOENT;
@@ -1086,7 +1087,7 @@ static void conf_ugid(char *runas, uid_t *uid, gid_t *gid)
 	warn("Don't run as root. Changing to nobody...");
 	{
 #ifndef GLIBC_NO_STATIC_NSS
-		struct passwd *pw;
+		const struct passwd *pw;
 		/* cppcheck-suppress getpwnamCalled */
 		pw = getpwnam("nobody");
 		if (!pw) {
@@ -1173,14 +1174,15 @@ void conf(struct ctx *c, int argc, char **argv)
 	bool copy_addrs_opt = false, copy_routes_opt = false;
 	enum port_fwd_mode fwd_default = FWD_NONE;
 	bool v4_only = false, v6_only = false;
-	char *runas = NULL, *logfile = NULL;
 	struct in6_addr *dns6 = c->ip6.dns;
 	struct fqdn *dnss = c->dns_search;
 	struct in_addr *dns4 = c->ip4.dns;
 	unsigned int ifi4 = 0, ifi6 = 0;
+	const char *logfile = NULL;
 	const char *optstring;
 	int name, ret, b, i;
 	size_t logsize = 0;
+	char *runas = NULL;
 	uid_t uid;
 	gid_t gid;
 
diff --git a/dhcp.c b/dhcp.c
index 53b4029..1107728 100644
--- a/dhcp.c
+++ b/dhcp.c
@@ -275,10 +275,10 @@ static void opt_set_dns_search(const struct ctx *c, size_t max_len)
 int dhcp(const struct ctx *c, const struct pool *p)
 {
 	size_t mlen, len, offset = 0, opt_len, opt_off = 0;
+	const struct ethhdr *eh;
+	const struct iphdr *iph;
+	const struct udphdr *uh;
 	struct in_addr mask;
-	struct ethhdr *eh;
-	struct iphdr *iph;
-	struct udphdr *uh;
 	unsigned int i;
 	struct msg *m;
 
@@ -312,7 +312,8 @@ int dhcp(const struct ctx *c, const struct pool *p)
 	offset += offsetof(struct msg, o);
 
 	while (opt_off + 2 < opt_len) {
-		uint8_t *olen, *type, *val;
+		const uint8_t *olen, *val;
+		uint8_t *type;
 
 		type = packet_get(p, 0, offset + opt_off,	1,	NULL);
 		olen = packet_get(p, 0, offset + opt_off + 1,	1,	NULL);
diff --git a/dhcpv6.c b/dhcpv6.c
index 58171bb..7dcca2a 100644
--- a/dhcpv6.c
+++ b/dhcpv6.c
@@ -426,10 +426,11 @@ search:
 int dhcpv6(struct ctx *c, const struct pool *p,
 	   const struct in6_addr *saddr, const struct in6_addr *daddr)
 {
-	struct opt_hdr *ia, *bad_ia, *client_id, *server_id;
-	struct in6_addr *src;
-	struct msg_hdr *mh;
-	struct udphdr *uh;
+	struct opt_hdr *ia, *bad_ia, *client_id;
+	const struct opt_hdr *server_id;
+	const struct in6_addr *src;
+	const struct msg_hdr *mh;
+	const struct udphdr *uh;
 	size_t mlen, n;
 
 	uh = packet_get(p, 0, 0, sizeof(*uh), &mlen);
diff --git a/log.c b/log.c
index b206f72..f71d0e2 100644
--- a/log.c
+++ b/log.c
@@ -222,7 +222,8 @@ void logfile_init(const char *name, const char *path, size_t size)
  */
 static void logfile_rotate_fallocate(int fd, const struct timespec *ts)
 {
-	char buf[BUFSIZ], *nl;
+	char buf[BUFSIZ];
+	const char *nl;
 	int n;
 
 	if (lseek(fd, 0, SEEK_SET) == -1)
@@ -260,7 +261,8 @@ static void logfile_rotate_fallocate(int fd, const struct timespec *ts)
 static void logfile_rotate_move(int fd, const struct timespec *ts)
 {
 	int header_len, write_offset, end, discard, n;
-	char buf[BUFSIZ], *nl;
+	char buf[BUFSIZ];
+	const char *nl;
 
 	header_len = snprintf(buf, BUFSIZ,
 			      "%s - log truncated at %lli.%04lli\n", log_header,
diff --git a/qrap.c b/qrap.c
index 1e5a802..97f350a 100644
--- a/qrap.c
+++ b/qrap.c
@@ -251,8 +251,8 @@ int main(int argc, char **argv)
 		}
 
 		if (!strcmp(argv[i], "-device") && i + 1 < argc) {
-			char *template = NULL;
-			char *p;
+			const char *template = NULL;
+			const char *p;
 
 			has_dev = 1;
 
diff --git a/tap.c b/tap.c
index 2ceda8d..396dee7 100644
--- a/tap.c
+++ b/tap.c
@@ -600,10 +600,10 @@ static int tap4_handler(struct ctx *c, const struct pool *in,
 resume:
 	for (seq_count = 0, seq = NULL; i < in->count; i++) {
 		size_t l2_len, l3_len, hlen, l4_len;
-		struct ethhdr *eh;
+		const struct ethhdr *eh;
+		const struct udphdr *uh;
 		struct iphdr *iph;
-		struct udphdr *uh;
-		char *l4h;
+		const char *l4h;
 
 		packet_get(in, i, 0, 0, &l2_len);
 
@@ -765,9 +765,9 @@ resume:
 	for (seq_count = 0, seq = NULL; i < in->count; i++) {
 		size_t l4_len, plen, check;
 		struct in6_addr *saddr, *daddr;
+		const struct ethhdr *eh;
+		const struct udphdr *uh;
 		struct ipv6hdr *ip6h;
-		struct ethhdr *eh;
-		struct udphdr *uh;
 		uint8_t proto;
 		char *l4h;
 
@@ -936,7 +936,7 @@ static void tap_sock_reset(struct ctx *c)
 void tap_handler_passt(struct ctx *c, uint32_t events,
 		       const struct timespec *now)
 {
-	struct ethhdr *eh;
+	const struct ethhdr *eh;
 	ssize_t n, rem;
 	char *p;
 
diff --git a/tcp.c b/tcp.c
index 9515649..5b37662 100644
--- a/tcp.c
+++ b/tcp.c
@@ -2296,7 +2296,7 @@ static int tcp_data_from_tap(struct ctx *c, struct tcp_tap_conn *conn,
 
 	for (i = idx, iov_i = 0; i < (int)p->count; i++) {
 		uint32_t seq, seq_offset, ack_seq;
-		struct tcphdr *th;
+		const struct tcphdr *th;
 		char *data;
 		size_t off;
 
@@ -2517,10 +2517,10 @@ int tcp_tap_handler(struct ctx *c, uint8_t pif, int af,
 		    const struct pool *p, int idx, const struct timespec *now)
 {
 	struct tcp_tap_conn *conn;
+	const struct tcphdr *th;
 	size_t optlen, len;
-	struct tcphdr *th;
+	const char *opts;
 	int ack_due = 0;
-	char *opts;
 	int count;
 
 	(void)pif;
@@ -3038,7 +3038,7 @@ void tcp_ns_sock_init(const struct ctx *c, in_port_t port)
  */
 static int tcp_ns_socks_init(void *arg)
 {
-	struct ctx *c = (struct ctx *)arg;
+	const struct ctx *c = (const struct ctx *)arg;
 	unsigned port;
 
 	ns_enter(c);
diff --git a/tcp_splice.c b/tcp_splice.c
index 1655f8e..0e2e04c 100644
--- a/tcp_splice.c
+++ b/tcp_splice.c
@@ -127,7 +127,7 @@ static int tcp_splice_epoll_ctl(const struct ctx *c,
 				struct tcp_splice_conn *conn)
 {
 	int m = conn->in_epoll ? EPOLL_CTL_MOD : EPOLL_CTL_ADD;
-	union epoll_ref ref[SIDES] = {
+	const union epoll_ref ref[SIDES] = {
 		{ .type = EPOLL_TYPE_TCP, .fd = conn->s[0],
 		  .flowside = FLOW_SIDX(conn, 0) },
 		{ .type = EPOLL_TYPE_TCP, .fd = conn->s[1],
diff --git a/udp.c b/udp.c
index 7057977..252d353 100644
--- a/udp.c
+++ b/udp.c
@@ -821,10 +821,10 @@ int udp_tap_handler(struct ctx *c, uint8_t pif,
 	struct iovec m[UIO_MAXIOV];
 	struct sockaddr_in6 s_in6;
 	struct sockaddr_in s_in;
+	const struct udphdr *uh;
 	struct sockaddr *sa;
 	int i, s, count = 0;
 	in_port_t src, dst;
-	struct udphdr *uh;
 	socklen_t sl;
 
 	(void)c;
@@ -1045,7 +1045,7 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
  * udp_sock_init_init() - Bind sockets in init namespace for inbound connections
  * @c:		Execution context
  */
-static void udp_sock_init_init(struct ctx *c)
+static void udp_sock_init_init(const struct ctx *c)
 {
 	unsigned dst;
 
@@ -1065,7 +1065,7 @@ static void udp_sock_init_init(struct ctx *c)
  */
 int udp_sock_init_ns(void *arg)
 {
-	struct ctx *c = (struct ctx *)arg;
+	const struct ctx *c = (const struct ctx *)arg;
 	unsigned dst;
 
 	ns_enter(c);
diff --git a/util.c b/util.c
index d51f50f..21b35ff 100644
--- a/util.c
+++ b/util.c
@@ -48,8 +48,8 @@
 char *ipv6_l4hdr(const struct pool *p, int idx, size_t offset, uint8_t *proto,
 		 size_t *dlen)
 {
-	struct ipv6_opt_hdr *o;
-	struct ipv6hdr *ip6h;
+	const struct ipv6_opt_hdr *o;
+	const struct ipv6hdr *ip6h;
 	char *base;
 	int hdrlen;
 	uint8_t nh;
-- 
@@ -48,8 +48,8 @@
 char *ipv6_l4hdr(const struct pool *p, int idx, size_t offset, uint8_t *proto,
 		 size_t *dlen)
 {
-	struct ipv6_opt_hdr *o;
-	struct ipv6hdr *ip6h;
+	const struct ipv6_opt_hdr *o;
+	const struct ipv6hdr *ip6h;
 	char *base;
 	int hdrlen;
 	uint8_t nh;
-- 
2.43.0

[PATCH 3/3] pif: Remove unused pif_name() function

[David Gibson]

pif_name() has no current callers, although we expect some as we expand the
flow table support.  I'm not sure why this didn't get caught by one of
our static checkers earlier, but it's now causing cppcheck failures for me.

Add a cppcheck suppression.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 pif.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/pif.h b/pif.h
index ca85b34..bd52936 100644
--- a/pif.h
+++ b/pif.h
@@ -38,6 +38,7 @@ static inline const char *pif_type(enum pif_type pt)
 		return "?";
 }
 
+/* cppcheck-suppress unusedFunction */
 static inline const char *pif_name(uint8_t pif)
 {
 	return pif_type(pif);
-- 
@@ -38,6 +38,7 @@ static inline const char *pif_type(enum pif_type pt)
 		return "?";
 }
 
+/* cppcheck-suppress unusedFunction */
 static inline const char *pif_name(uint8_t pif)
 {
 	return pif_type(pif);
-- 
2.43.0

Re: [PATCH 0/3] Assorted fixes

[Stefano Brivio]

On Mon, 15 Jan 2024 17:39:41 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> Came back from holidays to find a few things broken by external
> changes.  Here are the fixes for them.
> 
> David Gibson (3):
>   test: Fix passt.mbuto for cases where /usr/sbin doesn't exist
>   treewide: Make a bunch of pointer variables pointers to const
>   pif: Remove unused pif_name() function

Applied.

-- 
Stefano