[PATCH] log: Don't duplicate messages on stderr before daemonising
Now that logging functions force printing messages to stderr before
passt forks to background, we'll have duplicate messages when running
from an interactive terminal, or if --stderr is passed, because at
some point we set LOG_PERROR in our __openlog() wrapper.
We could defer setting LOG_PERROR, but that would change option
semantics in other, unexpected ways. We could force calling
passt_vsyslog() as long as the mask is set to LOG_EMERG, but that
complicates the logic in logging functions even further.
Go the easy way for now: don't force printing to stderr with
LOG_EMERG if LOG_PERROR is already set. We should seriously consider a
rework of those logging functions at this point.
Signed-off-by: Stefano Brivio
On Wed, Feb 15, 2023 at 12:55:29PM +0100, Stefano Brivio wrote:
Now that logging functions force printing messages to stderr before passt forks to background, we'll have duplicate messages when running from an interactive terminal, or if --stderr is passed, because at some point we set LOG_PERROR in our __openlog() wrapper.
We could defer setting LOG_PERROR, but that would change option semantics in other, unexpected ways. We could force calling passt_vsyslog() as long as the mask is set to LOG_EMERG, but that complicates the logic in logging functions even further.
Go the easy way for now: don't force printing to stderr with LOG_EMERG if LOG_PERROR is already set. We should seriously consider a rework of those logging functions at this point.
Signed-off-by: Stefano Brivio
Reviewed-by: David Gibson
--- This applies on top of Laine's series.
log.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/log.c b/log.c index 785bc36..f23e8ef 100644 --- a/log.c +++ b/log.c @@ -44,6 +44,8 @@ static char log_header[BUFSIZ]; /* File header, written back on cuts */ static time_t log_start; /* Start timestamp */ int log_trace; /* --trace mode enabled */
+#define BEFORE_DAEMON (setlogmask(0) == LOG_MASK(LOG_EMERG)) + #define logfn(name, level, doexit) \ void name(const char *format, ...) { \ struct timespec tp; \ @@ -56,8 +58,7 @@ void name(const char *format, ...) { \ tp.tv_nsec / (100L * 1000)); \ } \ \ - if ((LOG_MASK(LOG_PRI(level)) & log_mask) || \ - setlogmask(0) == LOG_MASK(LOG_EMERG)) { \ + if ((LOG_MASK(LOG_PRI(level)) & log_mask) || BEFORE_DAEMON) { \ va_start(args, format); \ if (log_file != -1) \ logfile_write(level, format, args); \ @@ -67,7 +68,7 @@ void name(const char *format, ...) { \ } \ \ if ((setlogmask(0) & LOG_MASK(LOG_DEBUG) && log_file == -1) || \ - setlogmask(0) == LOG_MASK(LOG_EMERG)) { \ + (BEFORE_DAEMON && !(log_opt & LOG_PERROR))) { \ va_start(args, format); \ (void)vfprintf(stderr, format, args); \ va_end(args); \
-- David Gibson | 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
participants (2)
-
David Gibson
-
Stefano Brivio