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=YjE5NAe6; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 989BB5A061A for ; Fri, 25 Oct 2024 03:04:29 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202410; t=1729818252; bh=aUF+HEGzXoQUrsjKv61VIRHkay9Z24hPtoOjNTOeAtA=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=YjE5NAe6NnrVxHNmCYw7YO3lSSa4YgyCaILPcdwYNTeQFjXIxiuInEizYOZRcLfTk n6NXgcOD5TqU9BtQbj859YO6w4PahUtxuI1F31f92JADifkwVSYlOJlqbHR/KApf+j HU9Ad+g3B7fSqo+rE8t9BGTuw+tDV803VyHO1RH7BUtVoXSsVBbn7lj6ju5+cnnt2E I8hojTE++XoTZz8OvMFHYIjSJg9qsE1w2qara0/dFNdVU+NaCJAA2q1BUvAaw/BO9a Fk8D0GxM6JSQBoKSWmt1k0DQiGdU6jGlLQTTsJrUxpIop0FMH0b366AGo/Dz8GznNP 2Acu5KROuInmg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4XZPgc0dl4z4x33; Fri, 25 Oct 2024 12:04:12 +1100 (AEDT) Date: Fri, 25 Oct 2024 12:00:26 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH 6/8] treewide: Address cert-err33-c clang-tidy warnings for clock and timer functions Message-ID: References: <20241024230438.3192725-1-sbrivio@redhat.com> <20241024230438.3192725-7-sbrivio@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="xeiGdWSqVFrex0H1" Content-Disposition: inline In-Reply-To: <20241024230438.3192725-7-sbrivio@redhat.com> Message-ID-Hash: NDEAITY3OQPV7RVAQO5XTT2CLS4DWOHA X-Message-ID-Hash: NDEAITY3OQPV7RVAQO5XTT2CLS4DWOHA 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: --xeiGdWSqVFrex0H1 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Oct 25, 2024 at 01:04:36AM +0200, 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 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, report an error, > there isn't much else we can do. >=20 > Signed-off-by: Stefano Brivio > --- > passt.c | 8 +++++--- > pcap.c | 12 ++++++------ > tcp.c | 12 +++++++++--- > 3 files changed, 20 insertions(+), 12 deletions(-) >=20 > diff --git a/passt.c b/passt.c > index ad6f0bc..e987f0d 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,7 @@ loop: > if (nfds =3D=3D -1 && errno !=3D EINTR) > die_perror("epoll_wait() failed in main loop"); > =20 > - clock_gettime(CLOCK_MONOTONIC, &now); > + (void)clock_gettime(CLOCK_MONOTONIC, &now); I think we should err() here. > =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 6753cfb..156c953 100644 > --- a/pcap.c > +++ b/pcap.c > @@ -100,12 +100,12 @@ 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); > + (void)clock_gettime(CLOCK_REALTIME, &now); =2E.and here.. > pcap_frame(&iov, 1, 0, &now); > } > =20 > @@ -119,13 +119,13 @@ 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); > + (void)clock_gettime(CLOCK_REALTIME, &now); =2E.and here.. > for (i =3D 0; i < n; i++) > pcap_frame(iov + i * frame_parts, frame_parts, offset, &now); > @@ -143,12 +143,12 @@ 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); > + (void)clock_gettime(CLOCK_REALTIME, &now); =2E.and here.. > pcap_frame(iov, iovcnt, offset, &now); > } > =20 > diff --git a/tcp.c b/tcp.c > index 0d22e07..d55caee 100644 > --- a/tcp.c > +++ b/tcp.c > @@ -548,7 +548,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 > /** > @@ -2240,7 +2241,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 > @@ -2278,7 +2281,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 --xeiGdWSqVFrex0H1 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmca7aoACgkQzQJF27ox 2GfqGQ//bUtx+NdFEwMKIrqsWiSTO6Fc/Hklb3FlLRCIJSaEyD3Mk7OH0PITfKW7 flnenP+f5mL1H6+/Lm+XqIDMtHpE3sIW4m9xa6zHCMWAosxV8tSehzcAnz1ngruQ hZ4OypdnCiVlhhLHpkfqiIXz4lX4XyKaS2Ok75z9u/oPkw1bpf+TcwLN0pCNM6AT G/C70qnD0Z0v85Av0vU41I9NnQeZxy3hCByBvp4urWJURpyAbdWKgNLx/9kqXcwE 2YuPuQGPOUCxSxvcGJt/8NfOkfMmL58nV5SN06pEHyDZlvD5MCMCH+YqPXN4bdFa SQlq3ihTwpFis/5VUV0nkl0YqllNlj0Np7MXuVJaERkcWp5GVew5+ods3p2NzLyi M9MwNkujLGEjzgC+xb/t6bZefMoaxFf6geyvWkxDkaGt2EK/dNAM6+HThUg0XUb5 MqL+yd536NJjMV+y4UnI/9pxYHK38eO7OBvKJmHu8JVl5yscGdotdx6iq1C4xEKg cu1MYXBtsQx3LMVguuq1lxsczRYDBTPhSrE8as7j7PK1KA+P7tj5X0iO6nfIG/bu qPPMBSmnsnV8CeEhNrueAAVjT+Mop4LRaxHMIRURKBoaIlCi6VlD1ybMd0iVCJTV FE5pgCn7CiLXPlSEL7MTS3buh82NwT3fVE3/jeS7pysMSj1c+4M= =nbDN -----END PGP SIGNATURE----- --xeiGdWSqVFrex0H1--