public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: passt-dev@passt.top, Stefano Brivio <sbrivio@redhat.com>
Cc: David Gibson <david@gibson.dropbear.id.au>
Subject: [PATCH v2 4/4] log: Avoid duplicate calls to logtime()
Date: Tue,  6 Aug 2024 16:18:39 +1000	[thread overview]
Message-ID: <20240806061839.1950144-5-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <20240806061839.1950144-1-david@gibson.dropbear.id.au>

We use logtime() to get a timestamp for the log in two places:
  - in vlogmsg(), which is used only for debug_print messages
  - in logfile_write() which is only used messages to the log file

These cases are mutually exclusive, so we don't ever print the same message
with different timestamps, but that's not particularly obvious to see.
It's possible future tweaks to logging logic could mean we log to two
different places with different timestamps, which would be confusing.

Refactor to have a single logtime() call in vlogmsg() and use it for all
the places we need it.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 log.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/log.c b/log.c
index 9f4cb7fc..f3799224 100644
--- a/log.c
+++ b/log.c
@@ -225,18 +225,16 @@ static int logfile_rotate(int fd, const struct timespec *now)
  * logfile_write() - Write entry to log file, trigger rotation if full
  * @newline:	Append newline at the end of the message, if missing
  * @pri:	Facility and level map, same as priority for vsyslog()
+ * @now:	Timestamp
  * @format:	Same as vsyslog() format
  * @ap:		Same as vsyslog() ap
  */
-static void logfile_write(bool newline, int pri, const char *format, va_list ap)
+static void logfile_write(bool newline, int pri, const struct timespec *now,
+			  const char *format, va_list ap)
 {
-	const struct timespec *now;
-	struct timespec ts;
 	char buf[BUFSIZ];
 	int n;
 
-	now = logtime(&ts);
-
 	n  = logtime_fmt(buf, BUFSIZ, now);
 	n += snprintf(buf + n, BUFSIZ - n, ": %s", logfile_prefix[pri]);
 
@@ -262,13 +260,14 @@ static void logfile_write(bool newline, int pri, const char *format, va_list ap)
 void vlogmsg(bool newline, int pri, const char *format, va_list ap)
 {
 	bool debug_print = (log_mask & LOG_MASK(LOG_DEBUG)) && log_file == -1;
+	const struct timespec *now;
+	struct timespec ts;
+
+	now = logtime(&ts);
 
 	if (debug_print) {
 		char timestr[LOGTIME_STRLEN];
-		const struct timespec *now;
-		struct timespec ts;
 
-		now = logtime(&ts);
 		logtime_fmt(timestr, sizeof(timestr), now);
 		fprintf(stderr, "%s: ", timestr);
 	}
@@ -278,7 +277,7 @@ void vlogmsg(bool newline, int pri, const char *format, va_list ap)
 
 		va_copy(ap2, ap); /* Don't clobber ap, we need it again */
 		if (log_file != -1)
-			logfile_write(newline, pri, format, ap2);
+			logfile_write(newline, pri, now, format, ap2);
 		else if (!(log_mask & LOG_MASK(LOG_DEBUG)))
 			passt_vsyslog(newline, pri, format, ap2);
 
-- 
@@ -225,18 +225,16 @@ static int logfile_rotate(int fd, const struct timespec *now)
  * logfile_write() - Write entry to log file, trigger rotation if full
  * @newline:	Append newline at the end of the message, if missing
  * @pri:	Facility and level map, same as priority for vsyslog()
+ * @now:	Timestamp
  * @format:	Same as vsyslog() format
  * @ap:		Same as vsyslog() ap
  */
-static void logfile_write(bool newline, int pri, const char *format, va_list ap)
+static void logfile_write(bool newline, int pri, const struct timespec *now,
+			  const char *format, va_list ap)
 {
-	const struct timespec *now;
-	struct timespec ts;
 	char buf[BUFSIZ];
 	int n;
 
-	now = logtime(&ts);
-
 	n  = logtime_fmt(buf, BUFSIZ, now);
 	n += snprintf(buf + n, BUFSIZ - n, ": %s", logfile_prefix[pri]);
 
@@ -262,13 +260,14 @@ static void logfile_write(bool newline, int pri, const char *format, va_list ap)
 void vlogmsg(bool newline, int pri, const char *format, va_list ap)
 {
 	bool debug_print = (log_mask & LOG_MASK(LOG_DEBUG)) && log_file == -1;
+	const struct timespec *now;
+	struct timespec ts;
+
+	now = logtime(&ts);
 
 	if (debug_print) {
 		char timestr[LOGTIME_STRLEN];
-		const struct timespec *now;
-		struct timespec ts;
 
-		now = logtime(&ts);
 		logtime_fmt(timestr, sizeof(timestr), now);
 		fprintf(stderr, "%s: ", timestr);
 	}
@@ -278,7 +277,7 @@ void vlogmsg(bool newline, int pri, const char *format, va_list ap)
 
 		va_copy(ap2, ap); /* Don't clobber ap, we need it again */
 		if (log_file != -1)
-			logfile_write(newline, pri, format, ap2);
+			logfile_write(newline, pri, now, format, ap2);
 		else if (!(log_mask & LOG_MASK(LOG_DEBUG)))
 			passt_vsyslog(newline, pri, format, ap2);
 
-- 
2.45.2


      parent reply	other threads:[~2024-08-06  6:18 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-06  6:18 [PATCH v2 0/4] Fix several small errors in log time handling David Gibson
2024-08-06  6:18 ` [PATCH v2 1/4] util: Some corrections for timespec_diff_us David Gibson
2024-08-06  6:18 ` [PATCH v2 2/4] log: Correct formatting of timestamps David Gibson
2024-08-06 19:08   ` Stefano Brivio
2024-08-07  1:06     ` David Gibson
2024-08-07  8:17       ` Stefano Brivio
2024-08-06  6:18 ` [PATCH v2 3/4] log: Handle errors from clock_gettime() David Gibson
2024-08-06  6:18 ` David Gibson [this message]

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=20240806061839.1950144-5-david@gibson.dropbear.id.au \
    --to=david@gibson.dropbear.id.au \
    --cc=passt-dev@passt.top \
    --cc=sbrivio@redhat.com \
    /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).