public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: Stefano Brivio <sbrivio@redhat.com>
To: passt-dev@passt.top
Cc: Yalan Zhang <yalzhang@redhat.com>,
	David Gibson <david@gibson.dropbear.id.au>
Subject: [PATCH v4 7/8] treewide: Replace strerror() calls
Date: Thu, 20 Jun 2024 06:07:59 +0200	[thread overview]
Message-ID: <20240620040800.3065684-8-sbrivio@redhat.com> (raw)
In-Reply-To: <20240620040800.3065684-1-sbrivio@redhat.com>

Now that we have logging functions embedding perror() functionality,
we can make _some_ calls more terse by using them. In many places,
the strerror() calls are still more convenient because, for example,
they are used in flow debugging functions, or because the return code
variable of interest is not 'errno'.

While at it, convert a few error messages from a scant perror style
to proper failure descriptions.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
 conf.c      |  6 +++---
 fwd.c       |  2 +-
 isolation.c | 28 +++++++++++-----------------
 log.c       |  2 +-
 netlink.c   |  4 ++--
 passt.c     | 12 ++++--------
 pasta.c     | 18 ++++++++----------
 pcap.c      |  8 +++-----
 tap.c       | 14 +++++++-------
 tcp.c       | 24 ++++++++----------------
 util.c      | 12 +++++-------
 11 files changed, 53 insertions(+), 77 deletions(-)

diff --git a/conf.c b/conf.c
index b8247c4..09bf6dd 100644
--- a/conf.c
+++ b/conf.c
@@ -461,7 +461,7 @@ static void get_dns(struct ctx *c)
 	}
 
 	if (line_len < 0)
-		warn("Error reading /etc/resolv.conf: %s", strerror(errno));
+		warn_perror("Error reading /etc/resolv.conf");
 	close(fd);
 
 out:
@@ -626,6 +626,7 @@ static unsigned int conf_ip4(unsigned int ifi,
 		int rc = nl_link_get_mac(nl_sock, ifi, mac);
 		if (rc < 0) {
 			char ifname[IFNAMSIZ];
+
 			err("Couldn't discover MAC address for %s: %s",
 			    if_indextoname(ifi, ifname), strerror(-rc));
 			return 0;
@@ -1549,8 +1550,7 @@ void conf(struct ctx *c, int argc, char **argv)
 				die("Redundant interface: %s", optarg);
 
 			if (!(ifi4 = ifi6 = if_nametoindex(optarg)))
-				die("Invalid interface name %s: %s", optarg,
-				    strerror(errno));
+				die_perror("Invalid interface name %s", optarg);
 			break;
 		case 'o':
 			if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr_out)	  &&
diff --git a/fwd.c b/fwd.c
index b3d5a37..d3f1798 100644
--- a/fwd.c
+++ b/fwd.c
@@ -52,7 +52,7 @@ static void procfs_scan_listen(int fd, unsigned int lstate,
 		return;
 
 	if (lseek(fd, 0, SEEK_SET)) {
-		warn("lseek() failed on /proc/net file: %s", strerror(errno));
+		warn_perror("lseek() failed on /proc/net file");
 		return;
 	}
 
diff --git a/isolation.c b/isolation.c
index c936674..4956d7e 100644
--- a/isolation.c
+++ b/isolation.c
@@ -105,7 +105,7 @@ static void drop_caps_ep_except(uint64_t keep)
 	int i;
 
 	if (syscall(SYS_capget, &hdr, data))
-		die("Couldn't get current capabilities: %s", strerror(errno));
+		die_perror("Couldn't get current capabilities");
 
 	for (i = 0; i < CAP_WORDS; i++) {
 		uint32_t mask = keep >> (32 * i);
@@ -115,7 +115,7 @@ static void drop_caps_ep_except(uint64_t keep)
 	}
 
 	if (syscall(SYS_capset, &hdr, data))
-		die("Couldn't drop capabilities: %s", strerror(errno));
+		die_perror("Couldn't drop capabilities");
 }
 
 /**
@@ -152,19 +152,17 @@ static void clamp_caps(void)
 		 */
 		if (prctl(PR_CAPBSET_DROP, i, 0, 0, 0) &&
 		    errno != EINVAL && errno != EPERM)
-			die("Couldn't drop cap %i from bounding set: %s",
-			    i, strerror(errno));
+			die_perror("Couldn't drop cap %i from bounding set", i);
 	}
 
 	if (syscall(SYS_capget, &hdr, data))
-		die("Couldn't get current capabilities: %s", strerror(errno));
+		die_perror("Couldn't get current capabilities");
 
 	for (i = 0; i < CAP_WORDS; i++)
 		data[i].inheritable = 0;
 
 	if (syscall(SYS_capset, &hdr, data))
-		die("Couldn't drop inheritable capabilities: %s",
-		    strerror(errno));
+		die_perror("Couldn't drop inheritable capabilities");
 }
 
 /**
@@ -234,34 +232,30 @@ void isolate_user(uid_t uid, gid_t gid, bool use_userns, const char *userns,
 	if (setgroups(0, NULL)) {
 		/* If we don't have CAP_SETGID, this will EPERM */
 		if (errno != EPERM)
-			die("Can't drop supplementary groups: %s",
-			    strerror(errno));
+			die_perror("Can't drop supplementary groups");
 	}
 
 	if (setgid(gid) != 0)
-		die("Can't set GID to %u: %s", gid, strerror(errno));
+		die_perror("Can't set GID to %u", gid);
 
 	if (setuid(uid) != 0)
-		die("Can't set UID to %u: %s", uid, strerror(errno));
+		die_perror("Can't set UID to %u", uid);
 
 	if (*userns) { /* If given a userns, join it */
 		int ufd;
 
 		ufd = open(userns, O_RDONLY | O_CLOEXEC);
 		if (ufd < 0)
-			die("Couldn't open user namespace %s: %s",
-			    userns, strerror(errno));
+			die_perror("Couldn't open user namespace %s", userns);
 
 		if (setns(ufd, CLONE_NEWUSER) != 0)
-			die("Couldn't enter user namespace %s: %s",
-			    userns, strerror(errno));
+			die_perror("Couldn't enter user namespace %s", userns);
 
 		close(ufd);
 
 	} else if (use_userns) { /* Create and join a new userns */
 		if (unshare(CLONE_NEWUSER) != 0)
-			die("Couldn't create user namespace: %s",
-			    strerror(errno));
+			die_perror("Couldn't create user namespace");
 	}
 
 	/* Joining a new userns gives us full capabilities; drop the
diff --git a/log.c b/log.c
index 0d7bfe2..270a790 100644
--- a/log.c
+++ b/log.c
@@ -212,7 +212,7 @@ void logfile_init(const char *name, const char *path, size_t size)
 	log_file = open(path, O_CREAT | O_TRUNC | O_APPEND | O_RDWR | O_CLOEXEC,
 			S_IRUSR | S_IWUSR);
 	if (log_file == -1)
-		die("Couldn't open log file %s: %s", path, strerror(errno));
+		die_perror("Couldn't open log file %s", path);
 
 	log_size = size ? size : LOGFILE_SIZE_DEFAULT;
 
diff --git a/netlink.c b/netlink.c
index 2c9e71f..3aed7e5 100644
--- a/netlink.c
+++ b/netlink.c
@@ -133,7 +133,7 @@ static uint32_t nl_send(int s, void *req, uint16_t type,
 
 	n = send(s, req, len, 0);
 	if (n < 0)
-		die("netlink: Failed to send(): %s", strerror(errno));
+		die_perror("netlink: Failed to send()");
 	else if (n < len)
 		die("netlink: Short send (%zd of %zd bytes)", n, len);
 
@@ -189,7 +189,7 @@ static struct nlmsghdr *nl_next(int s, char *buf, struct nlmsghdr *nh, ssize_t *
 
 	*n = recv(s, buf, NLBUFSIZ, 0);
 	if (*n < 0)
-		die("netlink: Failed to recv(): %s", strerror(errno));
+		die_perror("netlink: Failed to recv()");
 
 	nh = (struct nlmsghdr *)buf;
 	if (!NLMSG_OK(nh, *n))
diff --git a/passt.c b/passt.c
index 63351f7..fde15f9 100644
--- a/passt.c
+++ b/passt.c
@@ -227,15 +227,11 @@ int main(int argc, char **argv)
 		__openlog("pasta", 0, LOG_DAEMON);
 
 		sa.sa_handler = pasta_child_handler;
-		if (sigaction(SIGCHLD, &sa, NULL)) {
-			die("Couldn't install signal handlers: %s",
-			    strerror(errno));
-		}
+		if (sigaction(SIGCHLD, &sa, NULL))
+			die_perror("Couldn't install signal handlers");
 
-		if (signal(SIGPIPE, SIG_IGN) == SIG_ERR) {
-			die("Couldn't set disposition for SIGPIPE: %s",
-			    strerror(errno));
-		}
+		if (signal(SIGPIPE, SIG_IGN) == SIG_ERR)
+			die_perror("Couldn't set disposition for SIGPIPE");
 
 		c.mode = MODE_PASTA;
 	} else if (strstr(name, "passt")) {
diff --git a/pasta.c b/pasta.c
index d08391f..084288c 100644
--- a/pasta.c
+++ b/pasta.c
@@ -138,17 +138,15 @@ void pasta_open_ns(struct ctx *c, const char *netns)
 	int nfd = -1;
 
 	nfd = open(netns, O_RDONLY | O_CLOEXEC);
-	if (nfd < 0) {
-		die("Couldn't open network namespace %s: %s",
-		    netns, strerror(errno));
-	}
+	if (nfd < 0)
+		die_perror("Couldn't open network namespace %s", netns);
 
 	c->pasta_netns_fd = nfd;
 
 	NS_CALL(ns_check, c);
 
 	if (c->pasta_netns_fd < 0)
-		die("Couldn't switch to pasta namespaces: %s", strerror(errno));
+		die_perror("Couldn't switch to pasta namespaces");
 
 	if (!c->no_netns_quit) {
 		char buf[PATH_MAX] = { 0 };
@@ -184,7 +182,7 @@ static int pasta_spawn_cmd(void *arg)
 
 	/* We run in a detached PID and mount namespace: mount /proc over */
 	if (mount("", "/proc", "proc", 0, NULL))
-		warn("Couldn't mount /proc: %s", strerror(errno));
+		warn_perror("Couldn't mount /proc");
 
 	if (write_file("/proc/sys/net/ipv4/ping_group_range", "0 0"))
 		warn("Cannot set ping_group_range, ICMP requests might fail");
@@ -265,7 +263,7 @@ void pasta_start_ns(struct ctx *c, uid_t uid, gid_t gid,
 
 	NS_CALL(pasta_wait_for_ns, c);
 	if (c->pasta_netns_fd < 0)
-		die("Failed to join network namespace: %s", strerror(errno));
+		die_perror("Failed to join network namespace");
 }
 
 /**
@@ -369,12 +367,12 @@ static int pasta_netns_quit_timer(void)
 	struct itimerspec it = { { 1, 0 }, { 1, 0 } }; /* one-second interval */
 
 	if (fd == -1) {
-		err("timerfd_create(): %s", strerror(errno));
+		err_perror("Failed to create timerfd for quit timer");
 		return -errno;
 	}
 
 	if (timerfd_settime(fd, 0, &it, NULL) < 0) {
-		err("timerfd_settime(): %s", strerror(errno));
+		err_perror("Failed to set interval for quit timer");
 		close(fd);
 		return -errno;
 	}
@@ -467,7 +465,7 @@ void pasta_netns_quit_timer_handler(struct ctx *c, union epoll_ref ref)
 
 	n = read(ref.fd, &expirations, sizeof(expirations));
 	if (n < 0)
-		die("Namespace watch timer read() error: %s", strerror(errno));
+		die_perror("Namespace watch timer read() error");
 	if ((size_t)n < sizeof(expirations))
 		warn("Namespace watch timer: short read(): %zi", n);
 
diff --git a/pcap.c b/pcap.c
index 507be2a..46cc4b0 100644
--- a/pcap.c
+++ b/pcap.c
@@ -89,10 +89,8 @@ static void pcap_frame(const struct iovec *iov, size_t iovcnt,
 	struct iovec hiov = { &h, sizeof(h) };
 
 	if (write_remainder(pcap_fd, &hiov, 1, 0) < 0 ||
-	    write_remainder(pcap_fd, iov, iovcnt, offset) < 0) {
-		debug("Cannot log packet, length %zu: %s",
-		      l2len, strerror(errno));
-	}
+	    write_remainder(pcap_fd, iov, iovcnt, offset) < 0)
+		debug_perror("Cannot log packet, length %zu", l2len);
 }
 
 /**
@@ -178,5 +176,5 @@ void pcap_init(struct ctx *c)
 	info("Saving packet capture to %s", c->pcap);
 
 	if (write(pcap_fd, &pcap_hdr, sizeof(pcap_hdr)) < 0)
-		warn("Cannot write PCAP header: %s", strerror(errno));
+		warn_perror("Cannot write PCAP header");
 }
diff --git a/tap.c b/tap.c
index c9aeff1..ec994a2 100644
--- a/tap.c
+++ b/tap.c
@@ -325,7 +325,7 @@ static size_t tap_send_frames_pasta(const struct ctx *c,
 		size_t framelen = iov_size(iov + i, bufs_per_frame);
 
 		if (rc < 0) {
-			debug("tap write: %s", strerror(errno));
+			debug_perror("tap write");
 
 			switch (errno) {
 			case EAGAIN:
@@ -387,7 +387,7 @@ static size_t tap_send_frames_passt(const struct ctx *c,
 		size_t rembufs = bufs_per_frame - (i % bufs_per_frame);
 
 		if (write_remainder(c->fd_tap, &iov[i], rembufs, buf_offset) < 0) {
-			err("tap: partial frame send: %s", strerror(errno));
+			err_perror("tap: partial frame send");
 			return i;
 		}
 		i += rembufs;
@@ -1122,7 +1122,7 @@ int tap_sock_unix_open(char *sock_path)
 	int i;
 
 	if (fd < 0)
-		die("UNIX socket: %s", strerror(errno));
+		die_perror("Failed to open UNIX domain socket");
 
 	for (i = 1; i < UNIX_SOCK_MAX; i++) {
 		char *path = addr.sun_path;
@@ -1135,7 +1135,7 @@ int tap_sock_unix_open(char *sock_path)
 
 		ex = socket(AF_UNIX, SOCK_STREAM | SOCK_NONBLOCK, 0);
 		if (ex < 0)
-			die("UNIX domain socket check: %s", strerror(errno));
+			die_perror("Failed to check for UNIX domain conflicts");
 
 		ret = connect(ex, (const struct sockaddr *)&addr, sizeof(addr));
 		if (!ret || (errno != ENOENT && errno != ECONNREFUSED &&
@@ -1155,7 +1155,7 @@ int tap_sock_unix_open(char *sock_path)
 	}
 
 	if (i == UNIX_SOCK_MAX)
-		die("UNIX socket bind: %s", strerror(errno));
+		die_perror("Failed to bind UNIX domain socket");
 
 	info("UNIX domain socket bound at %s", addr.sun_path);
 	if (!*sock_path)
@@ -1261,11 +1261,11 @@ static int tap_ns_tun(void *arg)
 
 	fd = open("/dev/net/tun", flags);
 	if (fd < 0)
-		die("Failed to open() /dev/net/tun: %s", strerror(errno));
+		die_perror("Failed to open() /dev/net/tun");
 
 	rc = ioctl(fd, TUNSETIFF, &ifr);
 	if (rc < 0)
-		die("TUNSETIFF failed: %s", strerror(errno));
+		die_perror("TUNSETIFF ioctl on /dev/net/tun failed");
 
 	if (!(c->pasta_ifi = if_nametoindex(c->pasta_ifn)))
 		die("Tap device opened but no network interface found");
diff --git a/tcp.c b/tcp.c
index a2e81d5..698e7ec 100644
--- a/tcp.c
+++ b/tcp.c
@@ -1553,19 +1553,15 @@ static void tcp_bind_outbound(const struct ctx *c, int s, sa_family_t af)
 				.sin_addr = c->ip4.addr_out,
 			};
 
-			if (bind(s, (struct sockaddr *)&addr4, sizeof(addr4))) {
-				debug("Can't bind IPv4 TCP socket address: %s",
-				      strerror(errno));
-			}
+			if (bind(s, (struct sockaddr *)&addr4, sizeof(addr4)))
+				debug_perror("IPv4 TCP socket address bind");
 		}
 
 		if (*c->ip4.ifname_out) {
 			if (setsockopt(s, SOL_SOCKET, SO_BINDTODEVICE,
 				       c->ip4.ifname_out,
-				       strlen(c->ip4.ifname_out))) {
-				debug("Can't bind IPv4 TCP socket to interface:"
-				      " %s", strerror(errno));
-			}
+				       strlen(c->ip4.ifname_out)))
+				debug_perror("IPv4 TCP socket interface bind");
 		}
 	} else if (af == AF_INET6) {
 		if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr_out)) {
@@ -1575,19 +1571,15 @@ static void tcp_bind_outbound(const struct ctx *c, int s, sa_family_t af)
 				.sin6_addr = c->ip6.addr_out,
 			};
 
-			if (bind(s, (struct sockaddr *)&addr6, sizeof(addr6))) {
-				debug("Can't bind IPv6 TCP socket address: %s",
-				      strerror(errno));
-			}
+			if (bind(s, (struct sockaddr *)&addr6, sizeof(addr6)))
+				debug_perror("IPv6 TCP socket address bind");
 		}
 
 		if (*c->ip6.ifname_out) {
 			if (setsockopt(s, SOL_SOCKET, SO_BINDTODEVICE,
 				       c->ip6.ifname_out,
-				       strlen(c->ip6.ifname_out))) {
-				debug("Can't bind IPv6 TCP socket to interface:"
-				      " %s", strerror(errno));
-			}
+				       strlen(c->ip6.ifname_out)))
+				debug_perror("IPv6 TCP socket interface bind");
 		}
 	}
 }
diff --git a/util.c b/util.c
index 77448ec..dd2e57f 100644
--- a/util.c
+++ b/util.c
@@ -315,7 +315,7 @@ void bitmap_or(uint8_t *dst, size_t size, const uint8_t *a, const uint8_t *b)
 void ns_enter(const struct ctx *c)
 {
 	if (setns(c->pasta_netns_fd, CLONE_NEWNET))
-		die("setns() failed entering netns: %s", strerror(errno));
+		die_perror("setns() failed entering netns");
 }
 
 /**
@@ -330,10 +330,8 @@ bool ns_is_init(void)
 	bool ret = true;
 	int fd;
 
-	if ((fd = open("/proc/self/uid_map", O_RDONLY | O_CLOEXEC)) < 0) {
-		die("Can't determine if we're in init namespace: %s",
-		    strerror(errno));
-	}
+	if ((fd = open("/proc/self/uid_map", O_RDONLY | O_CLOEXEC)) < 0)
+		die_perror("Can't determine if we're in init namespace");
 
 	if (read(fd, buf, sizeof(root_uid_map)) != sizeof(root_uid_map) - 1 ||
 	    strncmp(buf, root_uid_map, sizeof(root_uid_map)))
@@ -509,7 +507,7 @@ int write_file(const char *path, const char *buf)
 	size_t len = strlen(buf);
 
 	if (fd < 0) {
-		warn("Could not open %s: %s", path, strerror(errno));
+		warn_perror("Could not open %s", path);
 		return -1;
 	}
 
@@ -517,7 +515,7 @@ int write_file(const char *path, const char *buf)
 		ssize_t rc = write(fd, buf, len);
 
 		if (rc <= 0) {
-			warn("Couldn't write to %s: %s", path, strerror(errno));
+			warn_perror("Couldn't write to %s", path);
 			break;
 		}
 
-- 
@@ -315,7 +315,7 @@ void bitmap_or(uint8_t *dst, size_t size, const uint8_t *a, const uint8_t *b)
 void ns_enter(const struct ctx *c)
 {
 	if (setns(c->pasta_netns_fd, CLONE_NEWNET))
-		die("setns() failed entering netns: %s", strerror(errno));
+		die_perror("setns() failed entering netns");
 }
 
 /**
@@ -330,10 +330,8 @@ bool ns_is_init(void)
 	bool ret = true;
 	int fd;
 
-	if ((fd = open("/proc/self/uid_map", O_RDONLY | O_CLOEXEC)) < 0) {
-		die("Can't determine if we're in init namespace: %s",
-		    strerror(errno));
-	}
+	if ((fd = open("/proc/self/uid_map", O_RDONLY | O_CLOEXEC)) < 0)
+		die_perror("Can't determine if we're in init namespace");
 
 	if (read(fd, buf, sizeof(root_uid_map)) != sizeof(root_uid_map) - 1 ||
 	    strncmp(buf, root_uid_map, sizeof(root_uid_map)))
@@ -509,7 +507,7 @@ int write_file(const char *path, const char *buf)
 	size_t len = strlen(buf);
 
 	if (fd < 0) {
-		warn("Could not open %s: %s", path, strerror(errno));
+		warn_perror("Could not open %s", path);
 		return -1;
 	}
 
@@ -517,7 +515,7 @@ int write_file(const char *path, const char *buf)
 		ssize_t rc = write(fd, buf, len);
 
 		if (rc <= 0) {
-			warn("Couldn't write to %s: %s", path, strerror(errno));
+			warn_perror("Couldn't write to %s", path);
 			break;
 		}
 
-- 
2.43.0


  parent reply	other threads:[~2024-06-20  4:08 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-20  4:07 [PATCH v4 0/8] Fixes for early logging/prints and related cleanups Stefano Brivio
2024-06-20  4:07 ` [PATCH v4 1/8] conf, passt: Don't try to log to stderr after we close it Stefano Brivio
2024-06-20  4:07 ` [PATCH v4 2/8] conf, passt: Make --stderr do nothing, and deprecate it Stefano Brivio
2024-06-20  4:07 ` [PATCH v4 3/8] conf, log: Instead of abusing log levels, add log_conf_parsed flag Stefano Brivio
2024-06-20  4:07 ` [PATCH v4 4/8] log, passt: Always print to stderr before initialisation is complete Stefano Brivio
2024-06-20  4:34   ` David Gibson
2024-06-20  4:07 ` [PATCH v4 5/8] log: Add _perror() logging function variants Stefano Brivio
2024-06-20  4:07 ` [PATCH v4 6/8] treewide: Replace perror() calls with calls to logging functions Stefano Brivio
2024-06-20  4:07 ` Stefano Brivio [this message]
2024-06-20  4:08 ` [PATCH v4 8/8] conf, passt: Don't call __openlog() if a log file is used Stefano Brivio

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240620040800.3065684-8-sbrivio@redhat.com \
    --to=sbrivio@redhat.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=passt-dev@passt.top \
    --cc=yalzhang@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).