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 v2 5/6] treewide: Replace perror() calls with calls to logging functions
Date: Tue, 18 Jun 2024 09:14:26 +0200	[thread overview]
Message-ID: <20240618071427.1544869-6-sbrivio@redhat.com> (raw)
In-Reply-To: <20240618071427.1544869-1-sbrivio@redhat.com>

perror() prints directly to standard error, but in many cases standard
error might be already closed, or we might want to skip logging, based
on configuration. Our logging functions provide all that.

While at it, make errors more descriptive, replacing some of the
existing basic perror-style messages.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 arch.c      | 10 +++++-----
 conf.c      |  6 ++----
 isolation.c | 18 ++++++++----------
 log.c       | 12 ++++--------
 passt.c     | 41 ++++++++++++++++-------------------------
 pasta.c     |  9 +++------
 6 files changed, 38 insertions(+), 58 deletions(-)

diff --git a/arch.c b/arch.c
index 80a41bc..04bebfc 100644
--- a/arch.c
+++ b/arch.c
@@ -18,6 +18,8 @@
 #include <string.h>
 #include <unistd.h>
 
+#include "log.h"
+
 /**
  * arch_avx2_exec() - Switch to AVX2 build if supported
  * @argv:	Arguments from command line
@@ -28,10 +30,8 @@ void arch_avx2_exec(char **argv)
 	char exe[PATH_MAX] = { 0 };
 	const char *p;
 
-	if (readlink("/proc/self/exe", exe, PATH_MAX - 1) < 0) {
-		perror("readlink /proc/self/exe");
-		exit(EXIT_FAILURE);
-	}
+	if (readlink("/proc/self/exe", exe, PATH_MAX - 1) < 0)
+		die_perror("Failed to read own /proc/self/exe link");
 
 	p = strstr(exe, ".avx2");
 	if (p && strlen(p) == strlen(".avx2"))
@@ -42,7 +42,7 @@ void arch_avx2_exec(char **argv)
 
 		snprintf(new_path, PATH_MAX + sizeof(".avx2"), "%s.avx2", exe);
 		execve(new_path, argv, environ);
-		perror("Can't run AVX2 build, using non-AVX2 version");
+		warn_perror("Can't run AVX2 build, using non-AVX2 version");
 	}
 }
 #else
diff --git a/conf.c b/conf.c
index 14feee1..344eb07 100644
--- a/conf.c
+++ b/conf.c
@@ -1098,10 +1098,8 @@ static void conf_ugid(char *runas, uid_t *uid, gid_t *gid)
 		const struct passwd *pw;
 		/* cppcheck-suppress getpwnamCalled */
 		pw = getpwnam("nobody");
-		if (!pw) {
-			perror("getpwnam");
-			exit(EXIT_FAILURE);
-		}
+		if (!pw)
+			die_perror("Can't get password file entry for nobody");
 
 		*uid = pw->pw_uid;
 		*gid = pw->pw_gid;
diff --git a/isolation.c b/isolation.c
index ca2c68b..c936674 100644
--- a/isolation.c
+++ b/isolation.c
@@ -316,34 +316,34 @@ int isolate_prefork(const struct ctx *c)
 		flags |= CLONE_NEWPID;
 
 	if (unshare(flags)) {
-		perror("unshare");
+		err_perror("Failed to detach isolating namespaces");
 		return -errno;
 	}
 
 	if (mount("", "/", "", MS_UNBINDABLE | MS_REC, NULL)) {
-		perror("mount /");
+		err_perror("Failed to remount /");
 		return -errno;
 	}
 
 	if (mount("", TMPDIR, "tmpfs",
 		  MS_NODEV | MS_NOEXEC | MS_NOSUID | MS_RDONLY,
 		  "nr_inodes=2,nr_blocks=0")) {
-		perror("mount tmpfs");
+		err_perror("Failed to mount empty tmpfs for pivot_root()");
 		return -errno;
 	}
 
 	if (chdir(TMPDIR)) {
-		perror("chdir");
+		err_perror("Failed to change directory into empty tmpfs");
 		return -errno;
 	}
 
 	if (syscall(SYS_pivot_root, ".", ".")) {
-		perror("pivot_root");
+		err_perror("Failed to pivot_root() into empty tmpfs");
 		return -errno;
 	}
 
 	if (umount2(".", MNT_DETACH | UMOUNT_NOFOLLOW)) {
-		perror("umount2");
+		err_perror("Failed to unmount original root filesystem");
 		return -errno;
 	}
 
@@ -388,8 +388,6 @@ void isolate_postfork(const struct ctx *c)
 	}
 
 	if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) ||
-	    prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog)) {
-		perror("prctl");
-		exit(EXIT_FAILURE);
-	}
+	    prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog))
+		die_perror("Failed to apply seccomp filter");
 }
diff --git a/log.c b/log.c
index 9ddc58c..ec833c4 100644
--- a/log.c
+++ b/log.c
@@ -208,10 +208,8 @@ void logfile_init(const char *name, const char *path, size_t size)
 	char nl = '\n', exe[PATH_MAX] = { 0 };
 	int n;
 
-	if (readlink("/proc/self/exe", exe, PATH_MAX - 1) < 0) {
-		perror("readlink /proc/self/exe");
-		exit(EXIT_FAILURE);
-	}
+	if (readlink("/proc/self/exe", exe, PATH_MAX - 1) < 0)
+		die_perror("Failed to read own /proc/self/exe link");
 
 	log_file = open(path, O_CREAT | O_TRUNC | O_APPEND | O_RDWR | O_CLOEXEC,
 			S_IRUSR | S_IWUSR);
@@ -224,10 +222,8 @@ void logfile_init(const char *name, const char *path, size_t size)
 		     name, exe, getpid());
 
 	if (write(log_file, log_header, n) <= 0 ||
-	    write(log_file, &nl, 1) <= 0) {
-		perror("Couldn't write to log file\n");
-		exit(EXIT_FAILURE);
-	}
+	    write(log_file, &nl, 1) <= 0)
+		die_perror("Couldn't write to log file");
 
 	/* For FALLOC_FL_COLLAPSE_RANGE: VFS block size can be up to one page */
 	log_cut_size = ROUND_UP(log_size * LOGFILE_CUT_RATIO / 100, PAGE_SIZE);
diff --git a/passt.c b/passt.c
index 7436120..542d3fb 100644
--- a/passt.c
+++ b/passt.c
@@ -136,14 +136,13 @@ static void secret_init(struct ctx *c)
 	}
 	if (dev_random >= 0)
 		close(dev_random);
-	if (random_read < sizeof(c->hash_secret)) {
+
+	if (random_read < sizeof(c->hash_secret))
 #else
 	if (getrandom(&c->hash_secret, sizeof(c->hash_secret),
-		      GRND_RANDOM) < 0) {
+		      GRND_RANDOM) < 0)
 #endif /* !HAS_GETRANDOM */
-		perror("TCP initial sequence getrandom");
-		exit(EXIT_FAILURE);
-	}
+		die_perror("Failed to get random bytes for hash table and TCP");
 }
 
 /**
@@ -250,20 +249,16 @@ int main(int argc, char **argv)
 	madvise(pkt_buf, TAP_BUF_BYTES, MADV_HUGEPAGE);
 
 	c.epollfd = epoll_create1(EPOLL_CLOEXEC);
-	if (c.epollfd == -1) {
-		perror("epoll_create1");
-		exit(EXIT_FAILURE);
-	}
+	if (c.epollfd == -1)
+		die_perror("Failed to create epoll file descriptor");
+
+	if (getrlimit(RLIMIT_NOFILE, &limit))
+		die_perror("Failed to get maximum value of open files limit");
 
-	if (getrlimit(RLIMIT_NOFILE, &limit)) {
-		perror("getrlimit");
-		exit(EXIT_FAILURE);
-	}
 	c.nofile = limit.rlim_cur = limit.rlim_max;
-	if (setrlimit(RLIMIT_NOFILE, &limit)) {
-		perror("setrlimit");
-		exit(EXIT_FAILURE);
-	}
+	if (setrlimit(RLIMIT_NOFILE, &limit))
+		die_perror("Failed to set current limit for open files");
+
 	sock_probe_mem(&c);
 
 	conf(&c, argc, argv);
@@ -293,10 +288,8 @@ int main(int argc, char **argv)
 	pcap_init(&c);
 
 	if (!c.foreground) {
-		if ((devnull_fd = open("/dev/null", O_RDWR | O_CLOEXEC)) < 0) {
-			perror("/dev/null open");
-			exit(EXIT_FAILURE);
-		}
+		if ((devnull_fd = open("/dev/null", O_RDWR | O_CLOEXEC)) < 0)
+			die_perror("Failed to open /dev/null");
 	}
 
 	if (isolate_prefork(&c))
@@ -324,10 +317,8 @@ loop:
 	/* NOLINTNEXTLINE(bugprone-branch-clone): intervals can be the same */
 	/* cppcheck-suppress [duplicateValueTernary, unmatchedSuppression] */
 	nfds = epoll_wait(c.epollfd, events, EPOLL_EVENTS, TIMER_INTERVAL);
-	if (nfds == -1 && errno != EINTR) {
-		perror("epoll_wait");
-		exit(EXIT_FAILURE);
-	}
+	if (nfds == -1 && errno != EINTR)
+		die_perror("epoll_wait() failed in main loop");
 
 	clock_gettime(CLOCK_MONOTONIC, &now);
 
diff --git a/pasta.c b/pasta.c
index b85ea2b..d08391f 100644
--- a/pasta.c
+++ b/pasta.c
@@ -197,8 +197,7 @@ static int pasta_spawn_cmd(void *arg)
 	a = (const struct pasta_spawn_cmd_arg *)arg;
 	execvp(a->exe, a->argv);
 
-	perror("execvp");
-	exit(EXIT_FAILURE);
+	die_perror("Failed to start command or shell");
 }
 
 /**
@@ -261,10 +260,8 @@ void pasta_start_ns(struct ctx *c, uid_t uid, gid_t gid,
 				   CLONE_NEWUTS | CLONE_NEWNS  | SIGCHLD,
 				   (void *)&arg);
 
-	if (pasta_child_pid == -1) {
-		perror("clone");
-		exit(EXIT_FAILURE);
-	}
+	if (pasta_child_pid == -1)
+		die_perror("Failed to clone process with detached namespaces");
 
 	NS_CALL(pasta_wait_for_ns, c);
 	if (c->pasta_netns_fd < 0)
-- 
@@ -197,8 +197,7 @@ static int pasta_spawn_cmd(void *arg)
 	a = (const struct pasta_spawn_cmd_arg *)arg;
 	execvp(a->exe, a->argv);
 
-	perror("execvp");
-	exit(EXIT_FAILURE);
+	die_perror("Failed to start command or shell");
 }
 
 /**
@@ -261,10 +260,8 @@ void pasta_start_ns(struct ctx *c, uid_t uid, gid_t gid,
 				   CLONE_NEWUTS | CLONE_NEWNS  | SIGCHLD,
 				   (void *)&arg);
 
-	if (pasta_child_pid == -1) {
-		perror("clone");
-		exit(EXIT_FAILURE);
-	}
+	if (pasta_child_pid == -1)
+		die_perror("Failed to clone process with detached namespaces");
 
 	NS_CALL(pasta_wait_for_ns, c);
 	if (c->pasta_netns_fd < 0)
-- 
2.43.0


  parent reply	other threads:[~2024-06-18  7:14 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-18  7:14 [PATCH v2 0/6] Fixes for early logging/prints and related cleanups Stefano Brivio
2024-06-18  7:14 ` [PATCH v2 1/6] conf, passt: Don't try to log to stderr after we close it Stefano Brivio
2024-06-19  2:14   ` David Gibson
2024-06-19  8:34     ` Stefano Brivio
2024-06-20  0:16       ` David Gibson
2024-06-18  7:14 ` [PATCH v2 2/6] conf, log: Instead of abusing log levels, add log_conf_parsed flag Stefano Brivio
2024-06-18  7:14 ` [PATCH v2 3/6] log, passt: Always print to stderr before initialisation is complete Stefano Brivio
2024-06-18  7:14 ` [PATCH v2 4/6] log: Add _perror() logging function variants Stefano Brivio
2024-06-19  2:21   ` David Gibson
2024-06-19  8:38     ` Stefano Brivio
2024-06-18  7:14 ` Stefano Brivio [this message]
2024-06-19  2:24   ` [PATCH v2 5/6] treewide: Replace perror() calls with calls to logging functions David Gibson
2024-06-18  7:14 ` [PATCH v2 6/6] treewide: Replace strerror() calls Stefano Brivio
2024-06-19  2:29   ` David Gibson
2024-06-19  8:36     ` 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=20240618071427.1544869-6-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).