Hi,

When writing my regression test for syslogd, I realized that with
LOG_CONS and LOG_PERROR trailing new lines are printed.  Of course
Perl Sys::Syslog that I use for the tests has bugs and appends '\n'
in cases where it should not.

But then I found this sentence in our man 3 syslog:

    A trailing newline is added if none is present.

There is no such check in our syslog_r(3) libc function.  It
unconditionally adds new lines when writing to stderr or console.
I would recommend to remove any trailing new line before adding it.
The syslog rfc does not want trailing new lines, our syslogd converts
them to spaces, and contrary to the documentation syslog_r(3) prints
empty lines.

While there, I found that LOG_CONS uses strchr() to find the beginning
of the string.  The code for LOG_PERROR saves a pointer.  I would
like to make a consistent implementation with conp and stdp pointer.
Also add a check that iov_len does not get negative, even if we
remove a new line.

And fix the compiler warning
/usr/src/lib/libc/gen/syslog_r.c:93: warning: 'stdp' may be used uninitialized 
in this function

ok?

bluhm

Index: lib/libc/gen/syslog_r.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/lib/libc/gen/syslog_r.c,v
retrieving revision 1.5
diff -u -p -r1.5 syslog_r.c
--- lib/libc/gen/syslog_r.c     14 Jul 2014 03:52:04 -0000      1.5
+++ lib/libc/gen/syslog_r.c     29 Aug 2014 20:33:00 -0000
@@ -90,7 +90,7 @@ __vsyslog_r(int pri, struct syslog_data 
        int fd, saved_errno, error;
 #define        TBUF_LEN        2048
 #define        FMT_LEN         1024
-       char *stdp, tbuf[TBUF_LEN], fmt_cpy[FMT_LEN];
+       char *conp = NULL, *stdp = NULL, tbuf[TBUF_LEN], fmt_cpy[FMT_LEN];
        int tbuf_left, fmt_left, prlen;
 
 #define        INTERNALLOG     LOG_ERR|LOG_CONS|LOG_PERROR|LOG_PID
@@ -127,6 +127,8 @@ __vsyslog_r(int pri, struct syslog_data 
 
        prlen = snprintf(p, tbuf_left, "<%d>", pri);
        DEC();
+       if (data->log_stat & LOG_CONS)
+               conp = p;
 
        /* 
         * syslogd will expand time automagically for reentrant case, and
@@ -195,13 +197,17 @@ __vsyslog_r(int pri, struct syslog_data 
        prlen = vsnprintf(p, tbuf_left, fmt_cpy, ap);
        DEC();
        cnt = p - tbuf;
+       if (cnt > 0 && p[-1] == '\n') {
+               *(--p) = '\0';
+               --cnt;
+       }
 
        /* Output to stderr if requested. */
        if (data->log_stat & LOG_PERROR) {
                struct iovec iov[2];
 
                iov[0].iov_base = stdp;
-               iov[0].iov_len = cnt - (stdp - tbuf);
+               iov[0].iov_len = cnt > stdp - tbuf ? cnt - (stdp - tbuf) : 0;
                iov[1].iov_base = "\n";
                iov[1].iov_len = 1;
                (void)writev(STDERR_FILENO, iov, 2);
@@ -222,9 +228,8 @@ __vsyslog_r(int pri, struct syslog_data 
            (fd = open(_PATH_CONSOLE, O_WRONLY|O_NONBLOCK, 0)) >= 0) {
                struct iovec iov[2];
                
-               p = strchr(tbuf, '>') + 1;
-               iov[0].iov_base = p;
-               iov[0].iov_len = cnt - (p - tbuf);
+               iov[0].iov_base = conp;
+               iov[0].iov_len = cnt > conp - tbuf ? cnt - (conp - tbuf) : 0;
                iov[1].iov_base = "\r\n";
                iov[1].iov_len = 2;
                (void)writev(fd, iov, 2);

Reply via email to