Re: [PATCH v2 9/9] convert all remaining err() followed by exit() to errexit()

[Stefano Brivio <sbrivio@redhat.com>]

On Wed,  8 Feb 2023 12:48:38 -0500
Laine Stump <laine@redhat.com> wrote:

> This actually leaves us with 0 uses of err(), but someone could want
> to use it in the future, so may as well leave it around.
> 
> Signed-off-by: Laine Stump <laine@redhat.com>
> ---
>  isolation.c | 78 +++++++++++++++++++----------------------------------
>  log.c       |  6 ++---
>  netlink.c   |  3 +--
>  passt.c     | 12 +++------
>  pasta.c     | 21 ++++++---------
>  tap.c       | 30 +++++++--------------
>  6 files changed, 53 insertions(+), 97 deletions(-)
> 
> diff --git a/isolation.c b/isolation.c
> index 4e6637d..0f709c6 100644
> --- a/isolation.c
> +++ b/isolation.c
> @@ -103,10 +103,8 @@ static void drop_caps_ep_except(uint64_t keep)
>  	struct __user_cap_data_struct data[CAP_WORDS];
>  	int i;
>  
> -	if (syscall(SYS_capget, &hdr, data)) {
> -		err("Couldn't get current capabilities: %s", strerror(errno));
> -		exit(EXIT_FAILURE);
> -	}
> +	if (syscall(SYS_capget, &hdr, data))
> +		errexit("Couldn't get current capabilities: %s", strerror(errno));

Split before strerror().

>  
>  	for (i = 0; i < CAP_WORDS; i++) {
>  		uint32_t mask = keep >> (32 * i);
> @@ -115,10 +113,8 @@ static void drop_caps_ep_except(uint64_t keep)
>  		data[i].permitted &= mask;
>  	}
>  
> -	if (syscall(SYS_capset, &hdr, data)) {
> -		err("Couldn't drop capabilities: %s", strerror(errno));
> -		exit(EXIT_FAILURE);
> -	}
> +	if (syscall(SYS_capset, &hdr, data))
> +		errexit("Couldn't drop capabilities: %s", strerror(errno));
>  }
>  
>  /**
> @@ -154,26 +150,20 @@ static void clamp_caps(void)
>  		 *   normal operation, so carry on without it.
>  		 */
>  		if (prctl(PR_CAPBSET_DROP, i, 0, 0, 0) &&
> -		    errno != EINVAL && errno != EPERM) {
> -			err("Couldn't drop cap %i from bounding set: %s",
> -			    i, strerror(errno));
> -			exit(EXIT_FAILURE);
> -		}
> +		    errno != EINVAL && errno != EPERM)
> +			errexit("Couldn't drop cap %i from bounding set: %s",
> +				i, strerror(errno));
>  	}
>  
> -	if (syscall(SYS_capget, &hdr, data)) {
> -		err("Couldn't get current capabilities: %s", strerror(errno));
> -		exit(EXIT_FAILURE);
> -	}
> +	if (syscall(SYS_capget, &hdr, data))
> +		errexit("Couldn't get current capabilities: %s", strerror(errno));

Same here.

>  
>  	for (i = 0; i < CAP_WORDS; i++)
>  		data[i].inheritable = 0;
>  
> -	if (syscall(SYS_capset, &hdr, data)) {
> -		err("Couldn't drop inheritable capabilities: %s",
> -		    strerror(errno));
> -		exit(EXIT_FAILURE);
> -	}
> +	if (syscall(SYS_capset, &hdr, data))
> +		errexit("Couldn't drop inheritable capabilities: %s",
> +			strerror(errno));
>  }
>  
>  /**
> @@ -229,46 +219,34 @@ void isolate_user(uid_t uid, gid_t gid, bool use_userns, const char *userns,
>  	/* First set our UID & GID in the original namespace */
>  	if (setgroups(0, NULL)) {
>  		/* If we don't have CAP_SETGID, this will EPERM */
> -		if (errno != EPERM) {
> -			err("Can't drop supplementary groups: %s",
> -			    strerror(errno));
> -			exit(EXIT_FAILURE);
> -		}
> +		if (errno != EPERM)
> +			errexit("Can't drop supplementary groups: %s",
> +				strerror(errno));
>  	}
>  
> -	if (setgid(gid) != 0) {
> -		err("Can't set GID to %u: %s", gid, strerror(errno));
> -		exit(EXIT_FAILURE);
> -	}
> +	if (setgid(gid) != 0)
> +		errexit("Can't set GID to %u: %s", gid, strerror(errno));
>  
> -	if (setuid(uid) != 0) {
> -		err("Can't set UID to %u: %s", uid, strerror(errno));
> -		exit(EXIT_FAILURE);
> -	}
> +	if (setuid(uid) != 0)
> +		errexit("Can't set UID to %u: %s", uid, strerror(errno));
>  
>  	if (*userns) { /* If given a userns, join it */
>  		int ufd;
>  
>  		ufd = open(userns, O_RDONLY | O_CLOEXEC);
> -		if (ufd < 0) {
> -			err("Couldn't open user namespace %s: %s",
> -			    userns, strerror(errno));
> -			exit(EXIT_FAILURE);
> -		}
> -
> -		if (setns(ufd, CLONE_NEWUSER) != 0) {
> -			err("Couldn't enter user namespace %s: %s",
> -			    userns, strerror(errno));
> -			exit(EXIT_FAILURE);
> -		}
> +		if (ufd < 0)
> +			errexit("Couldn't open user namespace %s: %s",
> +				userns, strerror(errno));
> +
> +		if (setns(ufd, CLONE_NEWUSER) != 0)
> +			errexit("Couldn't enter user namespace %s: %s",
> +				userns, strerror(errno));
>  
>  		close(ufd);
>  
>  	} else if (use_userns) { /* Create and join a new userns */
> -		if (unshare(CLONE_NEWUSER) != 0) {
> -			err("Couldn't create user namespace: %s", strerror(errno));
> -			exit(EXIT_FAILURE);
> -		}
> +		if (unshare(CLONE_NEWUSER) != 0)
> +			errexit("Couldn't create user namespace: %s", strerror(errno));

And here.

>  	}
>  
>  	/* Joining a new userns gives us full capabilities; drop the
> diff --git a/log.c b/log.c
> index 4956914..983c82f 100644
> --- a/log.c
> +++ b/log.c
> @@ -204,10 +204,8 @@ 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) {
> -		err("Couldn't open log file %s: %s", path, strerror(errno));
> -		exit(EXIT_FAILURE);
> -	}
> +	if (log_file == -1)
> +		errexit("Couldn't open log file %s: %s", path, strerror(errno));
>  
>  	log_size = size ? size : LOGFILE_SIZE_DEFAULT;
>  
> diff --git a/netlink.c b/netlink.c
> index 0850cbe..a6c6e1e 100644
> --- a/netlink.c
> +++ b/netlink.c
> @@ -90,8 +90,7 @@ void nl_sock_init(const struct ctx *c, bool ns)
>  	return;
>  
>  fail:
> -	err("Failed to get netlink socket");
> -	exit(EXIT_FAILURE);
> +	errexit("Failed to get netlink socket");
>  }
>  
>  /**
> diff --git a/passt.c b/passt.c
> index cf010e8..443d51a 100644
> --- a/passt.c
> +++ b/passt.c
> @@ -202,10 +202,8 @@ int main(int argc, char **argv)
>  	name = basename(argv0);
>  	if (strstr(name, "pasta")) {
>  		sa.sa_handler = pasta_child_handler;
> -		if (sigaction(SIGCHLD, &sa, NULL) || signal(SIGPIPE, SIG_IGN)) {
> -			err("Couldn't install signal handlers");
> -			exit(EXIT_FAILURE);
> -		}
> +		if (sigaction(SIGCHLD, &sa, NULL) || signal(SIGPIPE, SIG_IGN))
> +			errexit("Couldn't install signal handlers");
>  
>  		c.mode = MODE_PASTA;
>  		log_name = "pasta";
> @@ -291,10 +289,8 @@ int main(int argc, char **argv)
>  		}
>  	}
>  
> -	if (isolate_prefork(&c)) {
> -		err("Failed to sandbox process, exiting\n");
> -		exit(EXIT_FAILURE);
> -	}
> +	if (isolate_prefork(&c))
> +		errexit("Failed to sandbox process, exiting\n");
>  
>  	if (!c.foreground) {
>  		__daemon(pidfile_fd, devnull_fd);
> diff --git a/pasta.c b/pasta.c
> index 528f02a..b1463c9 100644
> --- a/pasta.c
> +++ b/pasta.c
> @@ -123,19 +123,15 @@ void pasta_open_ns(struct ctx *c, const char *netns)
>  	int nfd = -1;
>  
>  	nfd = open(netns, O_RDONLY | O_CLOEXEC);
> -	if (nfd < 0) {
> -		err("Couldn't open network namespace %s", netns);
> -		exit(EXIT_FAILURE);
> -	}
> +	if (nfd < 0)
> +		errexit("Couldn't open network namespace %s", netns);
>  
>  	c->pasta_netns_fd = nfd;
>  
>  	NS_CALL(ns_check, c);
>  
> -	if (c->pasta_netns_fd < 0) {
> -		err("Couldn't switch to pasta namespaces");
> -		exit(EXIT_FAILURE);
> -	}
> +	if (c->pasta_netns_fd < 0)
> +		errexit("Couldn't switch to pasta namespaces");
>  
>  	if (!c->no_netns_quit) {
>  		char buf[PATH_MAX] = { 0 };
> @@ -217,11 +213,10 @@ void pasta_start_ns(struct ctx *c, uid_t uid, gid_t gid,
>  			arg.exe = "/bin/sh";
>  
>  		if ((size_t)snprintf(sh_arg0, sizeof(sh_arg0),
> -				     "-%s", arg.exe) >= sizeof(sh_arg0)) {
> -			err("$SHELL is too long (%u bytes)",
> -			    strlen(arg.exe));
> -			exit(EXIT_FAILURE);
> -		}
> +				     "-%s", arg.exe) >= sizeof(sh_arg0))
> +			errexit("$SHELL is too long (%u bytes)",
> +				strlen(arg.exe));
> +
>  		sh_argv[0] = sh_arg0;
>  		arg.argv = sh_argv;
>  	}
> diff --git a/tap.c b/tap.c
> index 2e603ed..edb3184 100644
> --- a/tap.c
> +++ b/tap.c
> @@ -886,10 +886,8 @@ static void tap_sock_unix_init(struct ctx *c)
>  	};
>  	int i;
>  
> -	if (fd < 0) {
> -		err("UNIX socket: %s", strerror(errno));
> -		exit(EXIT_FAILURE);
> -	}
> +	if (fd < 0)
> +		errexit("UNIX socket: %s", strerror(errno));
>  
>  	/* In passt mode, we don't know the guest's MAC until it sends
>  	 * us packets.  Use the broadcast address so our first packets
> @@ -907,18 +905,14 @@ static void tap_sock_unix_init(struct ctx *c)
>  			snprintf(path, UNIX_PATH_MAX - 1, UNIX_SOCK_PATH, i);
>  
>  		ex = socket(AF_UNIX, SOCK_STREAM | SOCK_NONBLOCK, 0);
> -		if (ex < 0) {
> -			err("UNIX domain socket check: %s", strerror(errno));
> -			exit(EXIT_FAILURE);
> -		}
> +		if (ex < 0)
> +			errexit("UNIX domain socket check: %s", strerror(errno));

And here.

>  
>  		ret = connect(ex, (const struct sockaddr *)&addr, sizeof(addr));
>  		if (!ret || (errno != ENOENT && errno != ECONNREFUSED &&
>  			     errno != EACCES)) {
> -			if (*c->sock_path) {
> -				err("Socket path %s already in use", path);
> -				exit(EXIT_FAILURE);
> -			}
> +			if (*c->sock_path)
> +				errexit("Socket path %s already in use", path);
>  
>  			close(ex);
>  			continue;
> @@ -931,10 +925,8 @@ static void tap_sock_unix_init(struct ctx *c)
>  			break;
>  	}
>  
> -	if (i == UNIX_SOCK_MAX) {
> -		err("UNIX socket bind: %s", strerror(errno));
> -		exit(EXIT_FAILURE);
> -	}
> +	if (i == UNIX_SOCK_MAX)
> +		errexit("UNIX socket bind: %s", strerror(errno));
>  
>  	info("UNIX domain socket bound at %s\n", addr.sun_path);
>  
> @@ -1037,10 +1029,8 @@ static void tap_sock_tun_init(struct ctx *c)
>  	struct epoll_event ev = { 0 };
>  
>  	NS_CALL(tap_ns_tun, c);
> -	if (tun_ns_fd == -1) {
> -		err("Failed to open tun socket in namespace");
> -		exit(EXIT_FAILURE);
> -	}
> +	if (tun_ns_fd == -1)
> +		errexit("Failed to open tun socket in namespace");
>  
>  	pasta_ns_conf(c);
>  

-- 
Stefano