From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202410 header.b=CsidWCIl; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id C70455A061D for ; Tue, 29 Oct 2024 05:25:36 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202410; t=1730175928; bh=oyo45KuHyyu5VhJWyEQxOvAL+bUGqQkk1dyAZEr6H0M=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=CsidWCIlv9Qi4Un/pwTEW7iyOpQQSG0+8tRPVZpqtHZLQp7AeTE8scnW5HRjEh272 R+rlCv63G8T2xxL875AAEdsvWseerGkDlFgLbgC3XJSqgSU74p/IZXmoMa5q6jXEKx +niLYHor1PWLIyJ9tXcVw/uF/nNs8Vm/uiXW+0H9xjgko1lKhw2rWvyevI82wDzzAg lLuMQ5kpdgf/iWE0OsBR6IloIIgJYE+xKYE0PNFcv9/J+bKzNSN/hDJ7rpiR6Tfroi U//I/TihV+u+dFDQ3ItvdZkBkqwJvanmoKel+2PHehTyzvwrCbEbQo4bAg2b5/IVuE Ey+rqar1bGyqg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4Xcxxz75cGz4x8T; Tue, 29 Oct 2024 15:25:27 +1100 (AEDT) Date: Tue, 29 Oct 2024 15:24:34 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v3 7/9] treewide: Address cert-err33-c clang-tidy warnings for clock and timer functions Message-ID: References: <20241028100044.939714-1-sbrivio@redhat.com> <20241028100044.939714-8-sbrivio@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="bJMKAtipGASdHltp" Content-Disposition: inline In-Reply-To: <20241028100044.939714-8-sbrivio@redhat.com> Message-ID-Hash: YBS7V2JV2RHHHM2QFLIILTYVLWAV4G7C X-Message-ID-Hash: YBS7V2JV2RHHHM2QFLIILTYVLWAV4G7C 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 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: --bJMKAtipGASdHltp Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Oct 28, 2024 at 11:00:42AM +0100, Stefano Brivio wrote: > For clock_gettime(), we shouldn't ignore errors if they happen at > initialisation phase, because something is seriously wrong and it's > not helpful if we proceed as if nothing happened. >=20 > As we're up and running, though, it's probably better to report the > error and use a stale value than to terminate altogether. Make sure > we use a zero value if we don't have a stale one somewhere. >=20 > For timerfd_gettime() and timerfd_settime() failures, just report an > error, there isn't much else we can do. >=20 > Signed-off-by: Stefano Brivio Reviewed-by: David Gibson > --- > passt.c | 9 ++++++--- > pcap.c | 17 +++++++++++------ > tcp.c | 12 +++++++++--- > 3 files changed, 26 insertions(+), 12 deletions(-) >=20 > diff --git a/passt.c b/passt.c > index ad6f0bc..eaf231d 100644 > --- a/passt.c > +++ b/passt.c > @@ -207,7 +207,8 @@ int main(int argc, char **argv) > struct timespec now; > struct sigaction sa; > =20 > - clock_gettime(CLOCK_MONOTONIC, &log_start); > + if (clock_gettime(CLOCK_MONOTONIC, &log_start)) > + die_perror("Failed to get CLOCK_MONOTONIC time"); > =20 > arch_avx2_exec(argv); > =20 > @@ -265,7 +266,8 @@ int main(int argc, char **argv) > =20 > secret_init(&c); > =20 > - clock_gettime(CLOCK_MONOTONIC, &now); > + if (clock_gettime(CLOCK_MONOTONIC, &now)) > + die_perror("Failed to get CLOCK_MONOTONIC time"); > =20 > flow_init(); > =20 > @@ -313,7 +315,8 @@ loop: > if (nfds =3D=3D -1 && errno !=3D EINTR) > die_perror("epoll_wait() failed in main loop"); > =20 > - clock_gettime(CLOCK_MONOTONIC, &now); > + if (clock_gettime(CLOCK_MONOTONIC, &now)) > + err_perror("Failed to get CLOCK_MONOTONIC time"); > =20 > for (i =3D 0; i < nfds; i++) { > union epoll_ref ref =3D *((union epoll_ref *)&events[i].data.u64); > diff --git a/pcap.c b/pcap.c > index a07eb33..7751ddc 100644 > --- a/pcap.c > +++ b/pcap.c > @@ -100,12 +100,14 @@ static void pcap_frame(const struct iovec *iov, siz= e_t iovcnt, > void pcap(const char *pkt, size_t l2len) > { > struct iovec iov =3D { (char *)pkt, l2len }; > - struct timespec now; > + struct timespec now =3D { 0 }; > =20 > if (pcap_fd =3D=3D -1) > return; > =20 > - clock_gettime(CLOCK_REALTIME, &now); > + if (clock_gettime(CLOCK_REALTIME, &now)) > + err_perror("Failed to get CLOCK_REALTIME time"); > + > pcap_frame(&iov, 1, 0, &now); > } > =20 > @@ -119,13 +121,14 @@ void pcap(const char *pkt, size_t l2len) > void pcap_multiple(const struct iovec *iov, size_t frame_parts, unsigned= int n, > size_t offset) > { > - struct timespec now; > + struct timespec now =3D { 0 }; > unsigned int i; > =20 > if (pcap_fd =3D=3D -1) > return; > =20 > - clock_gettime(CLOCK_REALTIME, &now); > + if (clock_gettime(CLOCK_REALTIME, &now)) > + err_perror("Failed to get CLOCK_REALTIME time"); > =20 > for (i =3D 0; i < n; i++) > pcap_frame(iov + i * frame_parts, frame_parts, offset, &now); > @@ -143,12 +146,14 @@ void pcap_multiple(const struct iovec *iov, size_t = frame_parts, unsigned int n, > /* cppcheck-suppress unusedFunction */ > void pcap_iov(const struct iovec *iov, size_t iovcnt, size_t offset) > { > - struct timespec now; > + struct timespec now =3D { 0 }; > =20 > if (pcap_fd =3D=3D -1) > return; > =20 > - clock_gettime(CLOCK_REALTIME, &now); > + if (clock_gettime(CLOCK_REALTIME, &now)) > + err_perror("Failed to get CLOCK_REALTIME time"); > + > pcap_frame(iov, iovcnt, offset, &now); > } > =20 > diff --git a/tcp.c b/tcp.c > index 0569dc6..f03243d 100644 > --- a/tcp.c > +++ b/tcp.c > @@ -549,7 +549,8 @@ static void tcp_timer_ctl(const struct ctx *c, struct= tcp_tap_conn *conn) > (unsigned long long)it.it_value.tv_sec, > (unsigned long long)it.it_value.tv_nsec / 1000 / 1000); > =20 > - timerfd_settime(conn->timer, 0, &it, NULL); > + if (timerfd_settime(conn->timer, 0, &it, NULL)) > + flow_err(conn, "failed to set timer: %s", strerror(errno)); > } > =20 > /** > @@ -2235,7 +2236,9 @@ void tcp_timer_handler(const struct ctx *c, union e= poll_ref ref) > * timer is currently armed, this event came from a previous setting, > * and we just set the timer to a new point in the future: discard it. > */ > - timerfd_gettime(conn->timer, &check_armed); > + if (timerfd_gettime(conn->timer, &check_armed)) > + flow_err(conn, "failed to read timer: %s", strerror(errno)); > + > if (check_armed.it_value.tv_sec || check_armed.it_value.tv_nsec) > return; > =20 > @@ -2273,7 +2276,10 @@ void tcp_timer_handler(const struct ctx *c, union = epoll_ref ref) > * case. This avoids having to preemptively reset the timer on > * ~ACK_TO_TAP_DUE or ~ACK_FROM_TAP_DUE. > */ > - timerfd_settime(conn->timer, 0, &new, &old); > + if (timerfd_settime(conn->timer, 0, &new, &old)) > + flow_err(conn, "failed to set timer: %s", > + strerror(errno)); > + > if (old.it_value.tv_sec =3D=3D ACT_TIMEOUT) { > flow_dbg(conn, "activity timeout"); > tcp_rst(c, conn); --=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 --bJMKAtipGASdHltp Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmcgY4EACgkQzQJF27ox 2Gdf2RAAhqM2IsWp+gg/RMjEO2ffiW/h6d+NXnhecWDwotF2/yZ4r00Fp2c6tY/P qJFEe/lNOfcoHIok1RxwY1KtzfNAEdI45+Y4vwVJxXHOrP3NT8c9KarcQ3HnV506 GQcKDCQ/0wxiDaNf4C0b44LE+YOuiaqHhu/eF2tFMgxUuOU3VosPjZvom5TQOlAf DR20ZUj0xxyXkkNBB7KxebkRKE7K18DgSyyLXB/7f7V53xLlUxg3dq0mK97IFUK2 PTxMPTv6uxwrTUdHgzQal2bDoWC9T8RzzBgZyejWYpuw95R2YqsrkUWkT4gxozMt 6Kj5IuvGU7oQ3cbUyuf6uouwsSq9CxuHEsKt7FaU6xaJBqHvyMPRRuRqpVWaRvls qRp53Nbq8UFcaFWdZY2iTSjCNucWCEswCK7xBXJtx3cTU8JFAC8zkzIhXQf9F3FT 04l1toXre+S9b9kvb/Ii3duYs6b9rOooEkW9bfMrBFvTtsvRbUY2bHpWgjqiWIDm uIYN0wmccMDKizZx9RMdG7QGxunawJJIPnYxTVF4NzFZMLY2hMUnB3LgZECKrdc7 wpyQ2ymtRLgUwnOoxcWFKWJF+B9dVMPgzOuvJfi4EnpzU5B81OF+jZJrPvx3R1Kh Ijp+NXCFiiXFT8evE/6WgamUYEGAMsJpsa9rdn79+nJRatNbUBY= =36jU -----END PGP SIGNATURE----- --bJMKAtipGASdHltp--