Michael Savage wrote:
> Here's a patch with less fragile parsing code.
Comments inline.
> Index: syslogd.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/syslogd/syslogd.c,v
> retrieving revision 1.177
> diff -u -p -r1.177 syslogd.c
> --- syslogd.c 20 Jul 2015 19:49:33 -0000 1.177
> +++ syslogd.c 12 Feb 2016 20:51:41 -0000
> @@ -1324,6 +1324,30 @@ usage(void)
> exit(1);
> }
A one- or two-sentence comment describing the function may be nice, as
it isn't immediately obvious what it does. Not entirely necessary
though.
> +size_t
> +parsepriority(const char *msg, int *pri)
Nitpick, but I'd probably slightly prefer parse_priority.
> +{
> + size_t nlen;
> + char buf[11];
> + const char *errstr;
> + int maybepri;
> +
> + if (*msg++ == '<') {
> + nlen = strspn(msg, "1234567890");
> + if (nlen > 0 && nlen < sizeof(buf) && msg[nlen] == '>') {
> + memcpy(buf, msg, nlen);
> + buf[nlen] = '\0';
The above two lines should probably be strlcpy(buf, msg, nlen+1).
> + maybepri = strtonum(buf, 0, INT_MAX, &errstr);
> + if (errstr == NULL) {
> + *pri = maybepri;
> + return nlen + 2;
> + }
> + }
> + }
> +
> + return 0;
> +}
> +
> /*
> * Take a raw input line, decode the message, and print the message
> * on the appropriate log files.
> @@ -1337,13 +1361,7 @@ printline(char *hname, char *msg)
> /* test for special codes */
> pri = DEFUPRI;
> p = msg;
> - if (*p == '<') {
> - pri = 0;
> - while (isdigit((unsigned char)*++p))
> - pri = 10 * pri + (*p - '0');
> - if (*p == '>')
> - ++p;
> - }
> + p += parsepriority(p, &pri);
> if (pri &~ (LOG_FACMASK|LOG_PRIMASK))
> pri = DEFUPRI;
>
> @@ -1374,19 +1392,16 @@ printsys(char *msg)
> {
> int c, pri, flags;
> char *lp, *p, *q, line[MAXLINE + 1];
> + size_t prilen;
>
> (void)snprintf(line, sizeof line, "%s: ", _PATH_UNIX);
> lp = line + strlen(line);
> for (p = msg; *p != '\0'; ) {
> flags = SYNC_FILE | ADDDATE; /* fsync file after write */
> pri = DEFSPRI;
> - if (*p == '<') {
> - pri = 0;
> - while (isdigit((unsigned char)*++p))
> - pri = 10 * pri + (*p - '0');
> - if (*p == '>')
> - ++p;
> - } else {
> + prilen = parsepriority(p, &pri);
> + p += prilen;
> + if (prilen == 0) {
> /* kernel printf's come out on console */
> flags |= IGN_CONS;
> }
>
Looking at the old code again, it seems like the '>' was effectively
optional, and a stray '<' could zero pri. Was this a bug or a feature?
Otherwise, looks good to me. Thanks again!
