On Tue, 6 Aug 2024 16:18:37 +1000
David Gibson <david(a)gibson.dropbear.id.au> wrote:
logtime_fmt_and_arg() is a rather odd macro,
producing both a format
string and an argument, which can only be used in quite specific printf()
like formulations. It also has a significant bug: it tries to display 4
digits after the decimal point (so down to tenths of milliseconds) using
%04i. But the field width in printf() is always a *minimum* not maximum
field width, so this will not truncate the given value, but will redisplay
the entire tenth-of-milliseconds difference again after the decimal point.
Weird, not for me (glibc 2.38). But anyway, yes, it's much better this
way, and definitely, it's specified as minimum width (I never noticed).
Just one nit:
Replace the macro with an snprintf() like
function which will format the
timestamp, and use an explicit % to correct the display.
Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
log.c | 36 ++++++++++++++++++++++++------------
1 file changed, 24 insertions(+), 12 deletions(-)
diff --git a/log.c b/log.c
index eb3a780a..e60852f7 100644
--- a/log.c
+++ b/log.c
@@ -46,14 +46,24 @@ int log_trace; /* --trace mode enabled */
bool log_conf_parsed; /* Logging options already parsed */
bool log_runtime; /* Daemonised, or ready in foreground */
+#define LL_STRLEN (sizeof("-9223372036854775808"))
+#define LOGTIME_STRLEN (LL_STRLEN + 5)
+
/**
- * logtime_fmt_and_arg() - Build format and arguments to print relative log time
- * @x: Current timestamp
+ * logtime_fmt() - Format timestamp into a string for the log
+ * @buf: Buffer into which to format the time
+ * @size: Size of @buf
+ * @ts: Time to format
+ *
+ * Return: number of characters written to @buf (excluding \0)
*/
-#define logtime_fmt_and_arg(x) \
- "%lli.%04lli", \
- (timespec_diff_us((x), &log_start) / 1000000LL), \
- (timespec_diff_us((x), &log_start) / 100LL)
+int logtime_fmt(char *buf, size_t size, const struct timespec *ts)
Shouldn't this be static?
The rest of the series looks good to me, so I can
also fix this up
on merge, as you prefer.
If you can fix on merge that would be great.