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 <sbrivio(a)redhat.com> --- 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); \ -- 2.35.1
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 <sbrivio(a)redhat.com>Reviewed-by: David Gibson <david(a)gibson.dropbear.id.au>--- 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