From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 705685A005A for ; Thu, 20 Jun 2024 02:49:32 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1718844566; bh=12IVFtFQFUk6qgEpftMqhpZiEdSPkud0anHqmhpF9fg=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=NEQwY0KUA60yXVkbyKbz1V5P1TgTHhiBm1me/MMLwVOvN6qmrEpVzaZaXN5YrzgNk kkqo//12A/R6BZeCjJ4W0HABq9znOwWr62ojuhC8VhaQQ5m8hqaRSfIsuFJPLwucv3 rEo/uV7c0K8W8N/o1gjjh+Yvn3PqzPqTy8aUIEYIE0RFl+jbAGj2pr4db1qY3yvlgl fI75uO5xtBaKsejRdxVVuTtF+CX/6wPN7Nal0rAdwRudfIzDThCEPKYLDmM6t7shMe 1Rq0QeKFFCUPAkj5quOeZJ4ooHXBMzPWX+/Ej16r+f9sEmBjIbESb2Ak9qe+v0nq4+ KwSgXruNaKxDQ== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4W4MMB5KFBz4x0K; Thu, 20 Jun 2024 10:49:26 +1000 (AEST) Date: Thu, 20 Jun 2024 10:48:14 +1000 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v3 7/8] treewide: Replace strerror() calls Message-ID: References: <20240619194028.2913930-1-sbrivio@redhat.com> <20240619194028.2913930-8-sbrivio@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="Oyu2FQ46Y5k5arpL" Content-Disposition: inline In-Reply-To: <20240619194028.2913930-8-sbrivio@redhat.com> Message-ID-Hash: SDPVOQACTTEZDQDPL4YCDMKJQKWVVBW5 X-Message-ID-Hash: SDPVOQACTTEZDQDPL4YCDMKJQKWVVBW5 X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top, Yalan Zhang X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --Oyu2FQ46Y5k5arpL Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Jun 19, 2024 at 09:40:27PM +0200, Stefano Brivio wrote: > 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'. >=20 > While at it, convert a few error messages from a scant perror style > to proper failure descriptions. >=20 > Signed-off-by: Stefano Brivio Reviewed-by: David Gibson > --- > 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(-) >=20 > 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) > } > =20 > if (line_len < 0) > - warn("Error reading /etc/resolv.conf: %s", strerror(errno)); > + warn_perror("Error reading /etc/resolv.conf"); > close(fd); > =20 > out: > @@ -626,6 +626,7 @@ static unsigned int conf_ip4(unsigned int ifi, > int rc =3D 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); > =20 > if (!(ifi4 =3D ifi6 =3D 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 lst= ate, > return; > =20 > 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; > } > =20 > 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; > =20 > if (syscall(SYS_capget, &hdr, data)) > - die("Couldn't get current capabilities: %s", strerror(errno)); > + die_perror("Couldn't get current capabilities"); > =20 > for (i =3D 0; i < CAP_WORDS; i++) { > uint32_t mask =3D keep >> (32 * i); > @@ -115,7 +115,7 @@ static void drop_caps_ep_except(uint64_t keep) > } > =20 > if (syscall(SYS_capset, &hdr, data)) > - die("Couldn't drop capabilities: %s", strerror(errno)); > + die_perror("Couldn't drop capabilities"); > } > =20 > /** > @@ -152,19 +152,17 @@ static void clamp_caps(void) > */ > if (prctl(PR_CAPBSET_DROP, i, 0, 0, 0) && > errno !=3D EINVAL && errno !=3D 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); > } > =20 > if (syscall(SYS_capget, &hdr, data)) > - die("Couldn't get current capabilities: %s", strerror(errno)); > + die_perror("Couldn't get current capabilities"); > =20 > for (i =3D 0; i < CAP_WORDS; i++) > data[i].inheritable =3D 0; > =20 > if (syscall(SYS_capset, &hdr, data)) > - die("Couldn't drop inheritable capabilities: %s", > - strerror(errno)); > + die_perror("Couldn't drop inheritable capabilities"); > } > =20 > /** > @@ -234,34 +232,30 @@ void isolate_user(uid_t uid, gid_t gid, bool use_us= erns, const char *userns, > if (setgroups(0, NULL)) { > /* If we don't have CAP_SETGID, this will EPERM */ > if (errno !=3D EPERM) > - die("Can't drop supplementary groups: %s", > - strerror(errno)); > + die_perror("Can't drop supplementary groups"); > } > =20 > if (setgid(gid) !=3D 0) > - die("Can't set GID to %u: %s", gid, strerror(errno)); > + die_perror("Can't set GID to %u", gid); > =20 > if (setuid(uid) !=3D 0) > - die("Can't set UID to %u: %s", uid, strerror(errno)); > + die_perror("Can't set UID to %u", uid); > =20 > if (*userns) { /* If given a userns, join it */ > int ufd; > =20 > ufd =3D 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); > =20 > if (setns(ufd, CLONE_NEWUSER) !=3D 0) > - die("Couldn't enter user namespace %s: %s", > - userns, strerror(errno)); > + die_perror("Couldn't enter user namespace %s", userns); > =20 > close(ufd); > =20 > } else if (use_userns) { /* Create and join a new userns */ > if (unshare(CLONE_NEWUSER) !=3D 0) > - die("Couldn't create user namespace: %s", > - strerror(errno)); > + die_perror("Couldn't create user namespace"); > } > =20 > /* Joining a new userns gives us full capabilities; drop the > diff --git a/log.c b/log.c > index 09a7155..b34c203 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 =3D open(path, O_CREAT | O_TRUNC | O_APPEND | O_RDWR | O_CLOEX= EC, > S_IRUSR | S_IWUSR); > if (log_file =3D=3D -1) > - die("Couldn't open log file %s: %s", path, strerror(errno)); > + die_perror("Couldn't open log file %s", path); > =20 > log_size =3D size ? size : LOGFILE_SIZE_DEFAULT; > =20 > 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 ty= pe, > =20 > n =3D 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); > =20 > @@ -189,7 +189,7 @@ static struct nlmsghdr *nl_next(int s, char *buf, str= uct nlmsghdr *nh, ssize_t * > =20 > *n =3D recv(s, buf, NLBUFSIZ, 0); > if (*n < 0) > - die("netlink: Failed to recv(): %s", strerror(errno)); > + die_perror("netlink: Failed to recv()"); > =20 > nh =3D (struct nlmsghdr *)buf; > if (!NLMSG_OK(nh, *n)) > diff --git a/passt.c b/passt.c > index 5d154e1..0ba208d 100644 > --- a/passt.c > +++ b/passt.c > @@ -227,15 +227,11 @@ int main(int argc, char **argv) > __openlog("pasta", 0, LOG_DAEMON); > =20 > sa.sa_handler =3D 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"); > =20 > - if (signal(SIGPIPE, SIG_IGN) =3D=3D SIG_ERR) { > - die("Couldn't set disposition for SIGPIPE: %s", > - strerror(errno)); > - } > + if (signal(SIGPIPE, SIG_IGN) =3D=3D SIG_ERR) > + die_perror("Couldn't set disposition for SIGPIPE"); > =20 > c.mode =3D 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 =3D -1; > =20 > nfd =3D 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); > =20 > c->pasta_netns_fd =3D nfd; > =20 > NS_CALL(ns_check, c); > =20 > if (c->pasta_netns_fd < 0) > - die("Couldn't switch to pasta namespaces: %s", strerror(errno)); > + die_perror("Couldn't switch to pasta namespaces"); > =20 > if (!c->no_netns_quit) { > char buf[PATH_MAX] =3D { 0 }; > @@ -184,7 +182,7 @@ static int pasta_spawn_cmd(void *arg) > =20 > /* 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"); > =20 > 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 g= id, > =20 > 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"); > } > =20 > /** > @@ -369,12 +367,12 @@ static int pasta_netns_quit_timer(void) > struct itimerspec it =3D { { 1, 0 }, { 1, 0 } }; /* one-second interval= */ > =20 > if (fd =3D=3D -1) { > - err("timerfd_create(): %s", strerror(errno)); > + err_perror("Failed to create timerfd for quit timer"); > return -errno; > } > =20 > 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, un= ion epoll_ref ref) > =20 > n =3D 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); > =20 > 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 =3D { &h, sizeof(h) }; > =20 > 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); > } > =20 > /** > @@ -178,5 +176,5 @@ void pcap_init(struct ctx *c) > info("Saving packet capture to %s", c->pcap); > =20 > 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 =3D iov_size(iov + i, bufs_per_frame); > =20 > if (rc < 0) { > - debug("tap write: %s", strerror(errno)); > + debug_perror("tap write"); > =20 > switch (errno) { > case EAGAIN: > @@ -387,7 +387,7 @@ static size_t tap_send_frames_passt(const struct ctx = *c, > size_t rembufs =3D bufs_per_frame - (i % bufs_per_frame); > =20 > 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 +=3D rembufs; > @@ -1122,7 +1122,7 @@ int tap_sock_unix_open(char *sock_path) > int i; > =20 > if (fd < 0) > - die("UNIX socket: %s", strerror(errno)); > + die_perror("Failed to open UNIX domain socket"); > =20 > for (i =3D 1; i < UNIX_SOCK_MAX; i++) { > char *path =3D addr.sun_path; > @@ -1135,7 +1135,7 @@ int tap_sock_unix_open(char *sock_path) > =20 > ex =3D 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"); > =20 > ret =3D connect(ex, (const struct sockaddr *)&addr, sizeof(addr)); > if (!ret || (errno !=3D ENOENT && errno !=3D ECONNREFUSED && > @@ -1155,7 +1155,7 @@ int tap_sock_unix_open(char *sock_path) > } > =20 > if (i =3D=3D UNIX_SOCK_MAX) > - die("UNIX socket bind: %s", strerror(errno)); > + die_perror("Failed to bind UNIX domain socket"); > =20 > info("UNIX domain socket bound at %s", addr.sun_path); > if (!*sock_path) > @@ -1261,11 +1261,11 @@ static int tap_ns_tun(void *arg) > =20 > fd =3D 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"); > =20 > rc =3D ioctl(fd, TUNSETIFF, &ifr); > if (rc < 0) > - die("TUNSETIFF failed: %s", strerror(errno)); > + die_perror("TUNSETIFF ioctl on /dev/net/tun failed"); > =20 > if (!(c->pasta_ifi =3D 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 =3D c->ip4.addr_out, > }; > =20 > - 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"); > } > =20 > 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 =3D=3D 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 =3D c->ip6.addr_out, > }; > =20 > - 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"); > } > =20 > 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"); > } > =20 > /** > @@ -330,10 +330,8 @@ bool ns_is_init(void) > bool ret =3D true; > int fd; > =20 > - if ((fd =3D 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 =3D open("/proc/self/uid_map", O_RDONLY | O_CLOEXEC)) < 0) > + die_perror("Can't determine if we're in init namespace"); > =20 > if (read(fd, buf, sizeof(root_uid_map)) !=3D 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 =3D strlen(buf); > =20 > if (fd < 0) { > - warn("Could not open %s: %s", path, strerror(errno)); > + warn_perror("Could not open %s", path); > return -1; > } > =20 > @@ -517,7 +515,7 @@ int write_file(const char *path, const char *buf) > ssize_t rc =3D write(fd, buf, len); > =20 > if (rc <=3D 0) { > - warn("Couldn't write to %s: %s", path, strerror(errno)); > + warn_perror("Couldn't write to %s", path); > break; > } > =20 --=20 David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson --Oyu2FQ46Y5k5arpL Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmZzfE0ACgkQzQJF27ox 2GcmkBAAlOSQKrexM8r+jzUgsIWtttFu9WvzmO9TwslOpwOCK68UavdFaZ9X9RN9 VFSgFNa91oD+8aCR6Pto6/nEOJJtFFhSMub6L6wQkJ+MZUcGmqCSew8m5mSDaTve wlUYI5ynqfNZDF81BsfpoosLuGCoH7wHNAnG2Ovq8DUsf23MWMQ+3/T54/JLcHmk lwyIGMTpVFAoEzfW1LsjeWU9uzzyCkvrUlblfuXr4etlkKM5o/jha/9ZUqVMdzwL XWihshCqVSbAAO8lhtzeDkqWQPqiNEtRSKtDtT8cknJtTcnNRVLG1S5GYVJOU+Q+ NAjdvtPkvVcOqEM4r8Wz/0gezP6J04n4I1uPDgcBszwxEDVT/AGYrsVOVr7Tvdq6 N3fQmn7744EZf88Ww2wLeLwuF1M9W+l7MCbfvO7LMVj7myHSEAwMxfsjbRNLaGIZ MknQF395swA1yXxyVl5mAXJgjndFspzWFVPs1E/p9pT721Q3Jezd3vZAKxorCZSr tCNYMKLH2dLBi7jf3sNRuvp7Nhz8IBkFGzRXcujYqltbkzkynY/THky6pu3+7rCM 4Zva544WNX1uM6q0HzWlIjJB/R6S3xLeejp41DNms4oLuJXByaDPRwtNkX4IPwkb r8Ve7nwWbWcG3nAH/UsD1jp2Giy7ZPm78UizQyXUvWMepQTY/GQ= =Yjnw -----END PGP SIGNATURE----- --Oyu2FQ46Y5k5arpL--