On Mon, Jun 17, 2024 at 02:03:18PM +0200, Stefano Brivio wrote: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. Signed-off-by: Stefano Brivio <sbrivio(a)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..41bea01 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("readlink /proc/self/exe"); 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 0b76da9..7042f92 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("getpwnam"); *uid = pw->pw_uid; *gid = pw->pw_gid; diff --git a/isolation.c b/isolation.c index ca2c68b..871bbac 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("unshare");If we're changing these, I'd be inclined to make them more complete error messages, e.g. err("Failed to create isolating namespace: %s", strerror(errno));return -errno; } if (mount("", "/", "", MS_UNBINDABLE | MS_REC, NULL)) { - perror("mount /"); + err_perror("mount /"); 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("mount tmpfs"); return -errno; } if (chdir(TMPDIR)) { - perror("chdir"); + err_perror("chdir"); return -errno; } if (syscall(SYS_pivot_root, ".", ".")) { - perror("pivot_root"); + err_perror("pivot_root"); return -errno; } if (umount2(".", MNT_DETACH | UMOUNT_NOFOLLOW)) { - perror("umount2"); + err_perror("umount2"); 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("prctl"); } diff --git a/log.c b/log.c index 6764b2b..4aa800d 100644 --- a/log.c +++ b/log.c @@ -218,10 +218,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("readlink /proc/self/exe"); log_file = open(path, O_CREAT | O_TRUNC | O_APPEND | O_RDWR | O_CLOEXEC, S_IRUSR | S_IWUSR); @@ -234,10 +232,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\n"); /* 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 fa86164..4bc4251 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("TCP initial sequence getrandom"); } /** @@ -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("epoll_create1"); + + if (getrlimit(RLIMIT_NOFILE, &limit)) + die_perror("getrlimit"); - 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("setrlimit"); + 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("/dev/null open"); } 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"); clock_gettime(CLOCK_MONOTONIC, &now); diff --git a/pasta.c b/pasta.c index b85ea2b..ac2f898 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("execvp"); } /** @@ -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("clone"); NS_CALL(pasta_wait_for_ns, c); if (c->pasta_netns_fd < 0)-- 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