From mboxrd@z Thu Jan 1 00:00:00 1970 Received: by passt.top (Postfix, from userid 1000) id CF5005A0319; Wed, 24 Jul 2024 23:50:21 +0200 (CEST) From: Stefano Brivio To: passt-dev@passt.top Subject: [PATCH 03/11] log: Drop newlines in the middle of the perror()-like messages Date: Wed, 24 Jul 2024 23:50:09 +0200 Message-ID: <20240724215021.3366863-4-sbrivio@redhat.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240724215021.3366863-1-sbrivio@redhat.com> References: <20240724215021.3366863-1-sbrivio@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Message-ID-Hash: RJ52A3UCIJF5RF7WRBEUCX5AUQUISR25 X-Message-ID-Hash: RJ52A3UCIJF5RF7WRBEUCX5AUQUISR25 X-MailFrom: sbrivio@passt.top X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Calling vlogmsg() twice from logmsg_perror() results in this beauty: $ ./pasta -i foo Invalid interface name foo : No such device because the first part of the message, corresponding to the first call, doesn't end with a newline, and vlogmsg() adds it. Given that we can't easily append an argument (error description) to a variadic list, add a 'newline' parameter to all the functions that currently add a newline if missing, and disable that on the first call to vlogmsg() from logmsg_perror(). Not very pretty but I can't think of any solution that's less messy than this. Signed-off-by: Stefano Brivio --- flow.c | 2 +- log.c | 34 ++++++++++++++++++++++------------ log.h | 18 +++++++++--------- 3 files changed, 32 insertions(+), 22 deletions(-) diff --git a/flow.c b/flow.c index d7d548d..bd5fa2b 100644 --- a/flow.c +++ b/flow.c @@ -279,7 +279,7 @@ void flow_log_(const struct flow_common *f, int pri, const char *fmt, ...) else type_or_state = FLOW_TYPE(f); - logmsg(pri, "Flow %u (%s): %s", flow_idx(f), type_or_state, msg); + logmsg(true, pri, "Flow %u (%s): %s", flow_idx(f), type_or_state, msg); } /** diff --git a/log.c b/log.c index 9ed6bc1..54483e7 100644 --- a/log.c +++ b/log.c @@ -46,7 +46,14 @@ int log_trace; /* --trace mode enabled */ bool log_conf_parsed; /* Logging options already parsed */ bool log_runtime; /* Daemonised, or ready in foreground */ -void vlogmsg(int pri, const char *format, va_list ap) +/** + * vlogmsg() - Print or send messages to log or output files as configured + * @newline: Append newline at the end of the message, if missing + * @pri: Facility and level map, same as priority for vsyslog() + * @format: Message + * @ap: Variable argument list + */ +void vlogmsg(bool newline, int pri, const char *format, va_list ap) { bool debug_print = (log_mask & LOG_MASK(LOG_DEBUG)) && log_file == -1; struct timespec tp; @@ -63,9 +70,9 @@ void vlogmsg(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(pri, format, ap2); + logfile_write(newline, pri, format, ap2); else if (!(log_mask & LOG_MASK(LOG_DEBUG))) - passt_vsyslog(pri, format, ap2); + passt_vsyslog(newline, pri, format, ap2); va_end(ap2); } @@ -73,22 +80,23 @@ void vlogmsg(int pri, const char *format, va_list ap) if (debug_print || !log_conf_parsed || (!log_runtime && (log_mask & LOG_MASK(LOG_PRI(pri))))) { (void)vfprintf(stderr, format, ap); - if (format[strlen(format)] != '\n') + if (newline && format[strlen(format)] != '\n') fprintf(stderr, "\n"); } } /** * logmsg() - vlogmsg() wrapper for variable argument lists + * @newline: Append newline at the end of the message, if missing * @pri: Facility and level map, same as priority for vsyslog() * @format: Message */ -void logmsg(int pri, const char *format, ...) +void logmsg(bool newline, int pri, const char *format, ...) { va_list ap; va_start(ap, format); - vlogmsg(pri, format, ap); + vlogmsg(newline, pri, format, ap); va_end(ap); } @@ -103,10 +111,10 @@ void logmsg_perror(int pri, const char *format, ...) va_list ap; va_start(ap, format); - vlogmsg(pri, format, ap); + vlogmsg(false, pri, format, ap); va_end(ap); - logmsg(pri, ": %s", strerror(errno_copy)); + logmsg(true, pri, ": %s", strerror(errno_copy)); } /* Prefixes for log file messages, indexed by priority */ @@ -174,11 +182,12 @@ void __setlogmask(int mask) /** * passt_vsyslog() - vsyslog() implementation not using heap memory + * @newline: Append newline at the end of the message, if missing * @pri: Facility and level map, same as priority for vsyslog() * @format: Same as vsyslog() format * @ap: Same as vsyslog() ap */ -void passt_vsyslog(int pri, const char *format, va_list ap) +void passt_vsyslog(bool newline, int pri, const char *format, va_list ap) { char buf[BUFSIZ]; int n; @@ -188,7 +197,7 @@ void passt_vsyslog(int pri, const char *format, va_list ap) n += vsnprintf(buf + n, BUFSIZ - n, format, ap); - if (format[strlen(format)] != '\n') + if (newline && format[strlen(format)] != '\n') n += snprintf(buf + n, BUFSIZ - n, "\n"); if (log_sock >= 0 && send(log_sock, buf, n, 0) != n && !log_runtime) @@ -360,11 +369,12 @@ 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() * @format: Same as vsyslog() format * @ap: Same as vsyslog() ap */ -void logfile_write(int pri, const char *format, va_list ap) +void logfile_write(bool newline, int pri, const char *format, va_list ap) { struct timespec now; char buf[BUFSIZ]; @@ -380,7 +390,7 @@ void logfile_write(int pri, const char *format, va_list ap) n += vsnprintf(buf + n, BUFSIZ - n, format, ap); - if (format[strlen(format)] != '\n') + if (newline && format[strlen(format)] != '\n') n += snprintf(buf + n, BUFSIZ - n, "\n"); if ((log_written + n >= log_size) && logfile_rotate(log_file, &now)) diff --git a/log.h b/log.h index 6f34e8d..51ddafa 100644 --- a/log.h +++ b/log.h @@ -13,16 +13,16 @@ #define LOGFILE_CUT_RATIO 30 /* When full, cut ~30% size */ #define LOGFILE_SIZE_MIN (5UL * MAX(BUFSIZ, PAGE_SIZE)) -void vlogmsg(int pri, const char *format, va_list ap); -void logmsg(int pri, const char *format, ...) - __attribute__((format(printf, 2, 3))); +void vlogmsg(bool newline, int pri, const char *format, va_list ap); +void logmsg(bool newline, int pri, const char *format, ...) + __attribute__((format(printf, 3, 4))); void logmsg_perror(int pri, const char *format, ...) __attribute__((format(printf, 2, 3))); -#define err(...) logmsg( LOG_ERR, __VA_ARGS__) -#define warn(...) logmsg( LOG_WARNING, __VA_ARGS__) -#define info(...) logmsg( LOG_INFO, __VA_ARGS__) -#define debug(...) logmsg( LOG_DEBUG, __VA_ARGS__) +#define err(...) logmsg(true, LOG_ERR, __VA_ARGS__) +#define warn(...) logmsg(true, LOG_WARNING, __VA_ARGS__) +#define info(...) logmsg(true, LOG_INFO, __VA_ARGS__) +#define debug(...) logmsg(true, LOG_DEBUG, __VA_ARGS__) #define err_perror(...) logmsg_perror( LOG_ERR, __VA_ARGS__) #define warn_perror(...) logmsg_perror( LOG_WARNING, __VA_ARGS__) @@ -54,8 +54,8 @@ void trace_init(int enable); void __openlog(const char *ident, int option, int facility); void logfile_init(const char *name, const char *path, size_t size); -void passt_vsyslog(int pri, const char *format, va_list ap); -void logfile_write(int pri, const char *format, va_list ap); +void passt_vsyslog(bool newline, int pri, const char *format, va_list ap); +void logfile_write(bool newline, int pri, const char *format, va_list ap); void __setlogmask(int mask); #endif /* LOG_H */ -- 2.43.0