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 CB5735A004E for ; Tue, 18 Jun 2024 02:52:06 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1718671915; bh=Cx7IAcSmOaWV+QizWdJAPVNr/pCzJzaACfEsQwaBmBI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=GXjLjX4UlkuDFvSB7nWXfihwTj3IofaL4O0qtUMPEjG8Kz3x1yWEZ+uW/aiT+KNZq OX0dub3uut+l8Qd9W190YLa5X4dF2vB8Ke+KLo0b42QwpS2+QTjZ9gz7LydEwhL3yg Fdxf+Za2EDwOJTQLa/6UGm9IZBnioKdW4jfDNRqrqR4+WSAaHtCioGISwdMtjTReKH WJaAzcH7wEqGL84qY0lJ3t4c5vmbpg1DzWndnccVdOgdVBrRX8DmPIJEYS+xm0mND2 /CsCbi9SqFc04Ns+9NfTshebFFGRouas4Mp3e1Dkr1y8OABpIT8quTh8hGl6qNaboR /MxIxDFs+610Q== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4W37Vz2J79z4x21; Tue, 18 Jun 2024 10:51:55 +1000 (AEST) Date: Tue, 18 Jun 2024 10:50:07 +1000 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH 5/6] treewide: Replace perror() calls with calls to logging functions Message-ID: References: <20240617120319.1206857-1-sbrivio@redhat.com> <20240617120319.1206857-6-sbrivio@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="J+9uKm3IeFmsPjnT" Content-Disposition: inline In-Reply-To: <20240617120319.1206857-6-sbrivio@redhat.com> Message-ID-Hash: JXJT6X6BF43BQFMRFQTMGTLXP5ZZ2V6U X-Message-ID-Hash: JXJT6X6BF43BQFMRFQTMGTLXP5ZZ2V6U 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: --J+9uKm3IeFmsPjnT Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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. >=20 > Signed-off-by: Stefano Brivio > --- > 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(-) >=20 > diff --git a/arch.c b/arch.c > index 80a41bc..41bea01 100644 > --- a/arch.c > +++ b/arch.c > @@ -18,6 +18,8 @@ > #include > #include > =20 > +#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] =3D { 0 }; > const char *p; > =20 > - 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"); > =20 > p =3D strstr(exe, ".avx2"); > if (p && strlen(p) =3D=3D strlen(".avx2")) > @@ -42,7 +42,7 @@ void arch_avx2_exec(char **argv) > =20 > 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 =3D getpwnam("nobody"); > - if (!pw) { > - perror("getpwnam"); > - exit(EXIT_FAILURE); > - } > + if (!pw) > + die_perror("getpwnam"); > =20 > *uid =3D pw->pw_uid; > *gid =3D 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 |=3D CLONE_NEWPID; > =20 > 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; > } > =20 > if (mount("", "/", "", MS_UNBINDABLE | MS_REC, NULL)) { > - perror("mount /"); > + err_perror("mount /"); > return -errno; > } > =20 > if (mount("", TMPDIR, "tmpfs", > MS_NODEV | MS_NOEXEC | MS_NOSUID | MS_RDONLY, > "nr_inodes=3D2,nr_blocks=3D0")) { > - perror("mount tmpfs"); > + err_perror("mount tmpfs"); > return -errno; > } > =20 > if (chdir(TMPDIR)) { > - perror("chdir"); > + err_perror("chdir"); > return -errno; > } > =20 > if (syscall(SYS_pivot_root, ".", ".")) { > - perror("pivot_root"); > + err_perror("pivot_root"); > return -errno; > } > =20 > if (umount2(".", MNT_DETACH | UMOUNT_NOFOLLOW)) { > - perror("umount2"); > + err_perror("umount2"); > return -errno; > } > =20 > @@ -388,8 +388,6 @@ void isolate_postfork(const struct ctx *c) > } > =20 > 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 =3D '\n', exe[PATH_MAX] =3D { 0 }; > int n; > =20 > - 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"); > =20 > log_file =3D open(path, O_CREAT | O_TRUNC | O_APPEND | O_RDWR | O_CLOEX= EC, > S_IRUSR | S_IWUSR); > @@ -234,10 +232,8 @@ void logfile_init(const char *name, const char *path= , size_t size) > name, exe, getpid()); > =20 > if (write(log_file, log_header, n) <=3D 0 || > - write(log_file, &nl, 1) <=3D 0) { > - perror("Couldn't write to log file\n"); > - exit(EXIT_FAILURE); > - } > + write(log_file, &nl, 1) <=3D 0) > + die_perror("Couldn't write to log file\n"); > =20 > /* For FALLOC_FL_COLLAPSE_RANGE: VFS block size can be up to one page */ > log_cut_size =3D 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 >=3D 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"); > } > =20 > /** > @@ -250,20 +249,16 @@ int main(int argc, char **argv) > madvise(pkt_buf, TAP_BUF_BYTES, MADV_HUGEPAGE); > =20 > c.epollfd =3D epoll_create1(EPOLL_CLOEXEC); > - if (c.epollfd =3D=3D -1) { > - perror("epoll_create1"); > - exit(EXIT_FAILURE); > - } > + if (c.epollfd =3D=3D -1) > + die_perror("epoll_create1"); > + > + if (getrlimit(RLIMIT_NOFILE, &limit)) > + die_perror("getrlimit"); > =20 > - if (getrlimit(RLIMIT_NOFILE, &limit)) { > - perror("getrlimit"); > - exit(EXIT_FAILURE); > - } > c.nofile =3D limit.rlim_cur =3D 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); > =20 > conf(&c, argc, argv); > @@ -293,10 +288,8 @@ int main(int argc, char **argv) > pcap_init(&c); > =20 > if (!c.foreground) { > - if ((devnull_fd =3D open("/dev/null", O_RDWR | O_CLOEXEC)) < 0) { > - perror("/dev/null open"); > - exit(EXIT_FAILURE); > - } > + if ((devnull_fd =3D open("/dev/null", O_RDWR | O_CLOEXEC)) < 0) > + die_perror("/dev/null open"); > } > =20 > if (isolate_prefork(&c)) > @@ -324,10 +317,8 @@ loop: > /* NOLINTNEXTLINE(bugprone-branch-clone): intervals can be the same */ > /* cppcheck-suppress [duplicateValueTernary, unmatchedSuppression] */ > nfds =3D epoll_wait(c.epollfd, events, EPOLL_EVENTS, TIMER_INTERVAL); > - if (nfds =3D=3D -1 && errno !=3D EINTR) { > - perror("epoll_wait"); > - exit(EXIT_FAILURE); > - } > + if (nfds =3D=3D -1 && errno !=3D EINTR) > + die_perror("epoll_wait"); > =20 > clock_gettime(CLOCK_MONOTONIC, &now); > =20 > 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 =3D (const struct pasta_spawn_cmd_arg *)arg; > execvp(a->exe, a->argv); > =20 > - perror("execvp"); > - exit(EXIT_FAILURE); > + die_perror("execvp"); > } > =20 > /** > @@ -261,10 +260,8 @@ void pasta_start_ns(struct ctx *c, uid_t uid, gid_t = gid, > CLONE_NEWUTS | CLONE_NEWNS | SIGCHLD, > (void *)&arg); > =20 > - if (pasta_child_pid =3D=3D -1) { > - perror("clone"); > - exit(EXIT_FAILURE); > - } > + if (pasta_child_pid =3D=3D -1) > + die_perror("clone"); > =20 > NS_CALL(pasta_wait_for_ns, c); > if (c->pasta_netns_fd < 0) --=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 --J+9uKm3IeFmsPjnT Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmZw2b4ACgkQzQJF27ox 2GdaGBAAqw4gZyfBXZSGqgPFZSbcgwaaLl8Cd6Ka7seX8ADeIBmv7F3Xrnk1UemL 8MAUJ4nwXujzJFyqKUVeMI9bv4DW5t6lKQxRw4NfgMysNDyT2EPzrN9Qkx6/F3EY G2IqRvowofG2qzFso81xPg3NjlIec9I6eelB2FZeGzBSiYESnuVNSDIB2HIiuwkY 3e2E3gyi3qV8Ta6XaHZjHc0R+zfTKdvdCb9OB0uz4MHAR1f82mPIkAQu+7nPFvHt fQW5LBcUhor8vRcn8K1wsoOQv/9+aVO8szqXIp1YwHbl/RXsBmnsZfa09L3GxQT2 6SbTMuZHbeOV9I9sO8YeKk+DeC+Ali5dZfpZvST1OS6AZ+0SYOHD7Z6fDayiVsOk oxSC6EI+TltOnVqUSWkb3UeIuCQivmk2QApmXRID98voWh8C2qktG2W1IgaZa4Jd bjAlJlQKwOblmHg6QJn4obYC21IyshkmEUfBlmSzpGiZI5INQiAwS/axrzfgjAYa e4rd35cIv39KS0vr2bF8mcm9GPszA5VkG7c2iPY5kY6tKS3hTy1wbUFO67mk/beF g3I7wnQRzm6PP0Zz2zkjK4ddO+D6HQk67laDAyrmNklvoSJ6KfmwxfmQOgXZ5gLN QLTaPuQMkwfiKZH2gEBSibRUW7qiZPUWvmN1/rjZZYiltS3g6BQ= =uYOR -----END PGP SIGNATURE----- --J+9uKm3IeFmsPjnT--