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: David Gibson <david@gibson.dropbear.id.au>
Subject: [PATCH v3 7/9] treewide: Address cert-err33-c clang-tidy warnings for clock and timer functions
Date: Mon, 28 Oct 2024 11:00:42 +0100	[thread overview]
Message-ID: <20241028100044.939714-8-sbrivio@redhat.com> (raw)
In-Reply-To: <20241028100044.939714-1-sbrivio@redhat.com>

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.

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.

For timerfd_gettime() and timerfd_settime() failures, just report an
error, there isn't much else we can do.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 passt.c |  9 ++++++---
 pcap.c  | 17 +++++++++++------
 tcp.c   | 12 +++++++++---
 3 files changed, 26 insertions(+), 12 deletions(-)

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;
 
-	clock_gettime(CLOCK_MONOTONIC, &log_start);
+	if (clock_gettime(CLOCK_MONOTONIC, &log_start))
+		die_perror("Failed to get CLOCK_MONOTONIC time");
 
 	arch_avx2_exec(argv);
 
@@ -265,7 +266,8 @@ int main(int argc, char **argv)
 
 	secret_init(&c);
 
-	clock_gettime(CLOCK_MONOTONIC, &now);
+	if (clock_gettime(CLOCK_MONOTONIC, &now))
+		die_perror("Failed to get CLOCK_MONOTONIC time");
 
 	flow_init();
 
@@ -313,7 +315,8 @@ loop:
 	if (nfds == -1 && errno != EINTR)
 		die_perror("epoll_wait() failed in main loop");
 
-	clock_gettime(CLOCK_MONOTONIC, &now);
+	if (clock_gettime(CLOCK_MONOTONIC, &now))
+		err_perror("Failed to get CLOCK_MONOTONIC time");
 
 	for (i = 0; i < nfds; i++) {
 		union epoll_ref ref = *((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, size_t iovcnt,
 void pcap(const char *pkt, size_t l2len)
 {
 	struct iovec iov = { (char *)pkt, l2len };
-	struct timespec now;
+	struct timespec now = { 0 };
 
 	if (pcap_fd == -1)
 		return;
 
-	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);
 }
 
@@ -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 = { 0 };
 	unsigned int i;
 
 	if (pcap_fd == -1)
 		return;
 
-	clock_gettime(CLOCK_REALTIME, &now);
+	if (clock_gettime(CLOCK_REALTIME, &now))
+		err_perror("Failed to get CLOCK_REALTIME time");
 
 	for (i = 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 = { 0 };
 
 	if (pcap_fd == -1)
 		return;
 
-	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);
 }
 
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);
 
-	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));
 }
 
 /**
@@ -2235,7 +2236,9 @@ void tcp_timer_handler(const struct ctx *c, union epoll_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;
 
@@ -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 == ACT_TIMEOUT) {
 			flow_dbg(conn, "activity timeout");
 			tcp_rst(c, conn);
-- 
@@ -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);
 
-	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));
 }
 
 /**
@@ -2235,7 +2236,9 @@ void tcp_timer_handler(const struct ctx *c, union epoll_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;
 
@@ -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 == ACT_TIMEOUT) {
 			flow_dbg(conn, "activity timeout");
 			tcp_rst(c, conn);
-- 
2.43.0


  parent reply	other threads:[~2024-10-28 10:00 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-28 10:00 [PATCH v3 0/9] Take care of clang-tidy warnings with LLVM >= 16 Stefano Brivio
2024-10-28 10:00 ` [PATCH v3 1/9] Makefile: Exclude qrap.c from clang-tidy checks Stefano Brivio
2024-10-28 10:00 ` [PATCH v3 2/9] treewide: Comply with CERT C rule ERR33-C for snprintf() Stefano Brivio
2024-10-28 10:00 ` [PATCH v3 3/9] treewide: Silence cert-err33-c clang-tidy warnings for fprintf() Stefano Brivio
2024-10-28 10:00 ` [PATCH v3 4/9] Makefile: Disable readability-math-missing-parentheses clang-tidy check Stefano Brivio
2024-10-28 10:00 ` [PATCH v3 5/9] log: Don't use O_APPEND at all Stefano Brivio
2024-10-29  4:20   ` David Gibson
2024-10-29  8:48     ` Stefano Brivio
2024-10-29  9:32       ` David Gibson
2024-10-29 10:23         ` Stefano Brivio
2024-10-30  2:33           ` David Gibson
2024-10-30 12:27             ` Stefano Brivio
2024-10-31  0:35               ` David Gibson
2024-10-28 10:00 ` [PATCH v3 6/9] treewide: Suppress clang-tidy warning if we already use O_CLOEXEC or if we can't Stefano Brivio
2024-10-29  4:24   ` David Gibson
2024-10-28 10:00 ` Stefano Brivio [this message]
2024-10-29  4:24   ` [PATCH v3 7/9] treewide: Address cert-err33-c clang-tidy warnings for clock and timer functions David Gibson
2024-10-28 10:00 ` [PATCH v3 8/9] udp: Take care of cert-int09-c clang-tidy warning for enum udp_iov_idx Stefano Brivio
2024-10-28 10:00 ` [PATCH v3 9/9] util: Don't use errno after a successful call in __daemon() 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=20241028100044.939714-8-sbrivio@redhat.com \
    --to=sbrivio@redhat.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=passt-dev@passt.top \
    /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).