log.c use buffered IO

[Claudio Jeker]

I dislike how log.c does all these asprintf() calls with dubious
workaround calls in case asprintf() fails.
IMO it is easier to use the stdio provided buffers and fflush() to get
"atomic" writes on stderr. At least from my understanding this is the
reason to go all this lenght to do a single fprintf call.
We need this since in privsep daemons can log from multiple processes
concurrently and we want the log to be not too mangled.

While there also use one static buffer to handle log_warn() and vfatalc()
where the error message is first expanded and then printed with
logit(LOG_ERR, "%s: %s", fmtbuf, strerror(saved_errno));

Any opinions?
-- 
:wq Claudio

Index: log.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/log.c,v
retrieving revision 1.64
diff -u -p -r1.64 log.c
--- log.c       21 Mar 2017 12:06:55 -0000      1.64
+++ log.c       16 Oct 2023 09:42:55 -0000
@@ -29,6 +29,7 @@
 static int              debug;
 static int              verbose;
 static const char      *log_procname;
+static char             logbuf[4096], fmtbuf[4096];
 
 void
 log_init(int n_debug, int facility)
@@ -41,6 +42,8 @@ log_init(int n_debug, int facility)
 
        if (!debug)
                openlog(__progname, LOG_PID | LOG_NDELAY, facility);
+       else
+               setvbuf(stderr, logbuf, _IOFBF, sizeof(logbuf));
 
        tzset();
 }
@@ -77,18 +80,11 @@ logit(int pri, const char *fmt, ...)
 void
 vlog(int pri, const char *fmt, va_list ap)
 {
-       char    *nfmt;
        int      saved_errno = errno;
 
        if (debug) {
-               /* best effort in out of mem situations */
-               if (asprintf(&nfmt, "%s\n", fmt) == -1) {
-                       vfprintf(stderr, fmt, ap);
-                       fprintf(stderr, "\n");
-               } else {
-                       vfprintf(stderr, nfmt, ap);
-                       free(nfmt);
-               }
+               vfprintf(stderr, fmt, ap);
+               fprintf(stderr, "\n");
                fflush(stderr);
        } else
                vsyslog(pri, fmt, ap);
@@ -99,7 +95,6 @@ vlog(int pri, const char *fmt, va_list a
 void
 log_warn(const char *emsg, ...)
 {
-       char            *nfmt;
        va_list          ap;
        int              saved_errno = errno;
 
@@ -108,16 +103,8 @@ log_warn(const char *emsg, ...)
                logit(LOG_ERR, "%s", strerror(saved_errno));
        else {
                va_start(ap, emsg);
-
-               if (asprintf(&nfmt, "%s: %s", emsg,
-                   strerror(saved_errno)) == -1) {
-                       /* we tried it... */
-                       vlog(LOG_ERR, emsg, ap);
-                       logit(LOG_ERR, "%s", strerror(saved_errno));
-               } else {
-                       vlog(LOG_ERR, nfmt, ap);
-                       free(nfmt);
-               }
+               (void)vsnprintf(fmtbuf, sizeof(fmtbuf), emsg, ap);
+               logit(LOG_ERR, "%s: %s", fmtbuf, strerror(saved_errno));
                va_end(ap);
        }
 
@@ -159,21 +146,20 @@ log_debug(const char *emsg, ...)
 static void
 vfatalc(int code, const char *emsg, va_list ap)
 {
-       static char     s[BUFSIZ];
        const char      *sep;
 
        if (emsg != NULL) {
-               (void)vsnprintf(s, sizeof(s), emsg, ap);
+               (void)vsnprintf(fmtbuf, sizeof(fmtbuf), emsg, ap);
                sep = ": ";
        } else {
-               s[0] = '\0';
+               fmtbuf[0] = '\0';
                sep = "";
        }
        if (code)
                logit(LOG_CRIT, "fatal in %s: %s%s%s",
-                   log_procname, s, sep, strerror(code));
+                   log_procname, fmtbuf, sep, strerror(code));
        else
-               logit(LOG_CRIT, "fatal in %s%s%s", log_procname, sep, s);
+               logit(LOG_CRIT, "fatal in %s%s%s", log_procname, sep, fmtbuf);
 }
 
 void