Page MenuHomeFreeBSD
Differential D26456 Diff 77303 usr.sbin/syslogd/syslogd.c

Changeset View

Standalone View

View Options

usr.sbin/syslogd/syslogd.c

Show First 20 Lines • Show All 127 Lines • ▼ Show 20 Lines
#include <ctype.h> #include <ctype.h>
#include <dirent.h> #include <dirent.h>
#include <err.h> #include <err.h>
#include <errno.h> #include <errno.h>
#include <fcntl.h> #include <fcntl.h>
#include <fnmatch.h> #include <fnmatch.h>
#include <libutil.h> #include <libutil.h>
#include <limits.h> #include <limits.h>
#include <locale.h>
#include <netdb.h> #include <netdb.h>
#include <paths.h> #include <paths.h>
#include <regex.h>
#include <signal.h> #include <signal.h>
#include <stdbool.h> #include <stdbool.h>
#include <stddef.h> #include <stddef.h>
#include <stdio.h> #include <stdio.h>
#include <stdlib.h> #include <stdlib.h>
#include <string.h> #include <string.h>
#include <sysexits.h> #include <sysexits.h>
#include <uchar.h>
#include <unistd.h> #include <unistd.h>
#include <utmpx.h> #include <utmpx.h>
#include <regex.h> #include <vis.h>
#include <wctype.h>
#include "pathnames.h" #include "pathnames.h"
#include "ttymsg.h" #include "ttymsg.h"
#define SYSLOG_NAMES #define SYSLOG_NAMES
#include <sys/syslog.h> #include <sys/syslog.h>
static const char *ConfFile = _PATH_LOGCONF; static const char *ConfFile = _PATH_LOGCONF;
▲ Show 20 Lines • Show All 623 Lines • ▼ Show 20 Lines#endif
(void)signal(SIGCHLD, sighandler); (void)signal(SIGCHLD, sighandler);
(void)signal(SIGALRM, domark); (void)signal(SIGALRM, domark);
(void)signal(SIGPIPE, SIG_IGN); /* We'll catch EPIPE instead. */ (void)signal(SIGPIPE, SIG_IGN); /* We'll catch EPIPE instead. */
(void)alarm(TIMERINTVL); (void)alarm(TIMERINTVL);
/* tuck my process id away */ /* tuck my process id away */
pidfile_write(pfh); pidfile_write(pfh);
/* set the character set locale */
setlocale(LC_CTYPE, "");
emasteUnsubmitted
Done
Inline Actions

Just setlocale(LC_CTYPE, "") probably.

Only three locales are defined by default, the empty string "" which
denotes the native environment, and the "C" and "POSIX" locales, which
denote the C language environment.  A locale argument of NULL causes
setlocale() to return the current locale.

If LC_CTYPE is unset getenv returns null, which causes setlocale() to just return the current setting. The empty string sets it according to environment variables automatically.

emaste: Just `setlocale(LC_CTYPE, "")` probably. ``` Only three locales are defined by default…
dprintf("off & running....\n"); dprintf("off & running....\n");
tvp = &tv; tvp = &tv;
tv.tv_sec = tv.tv_usec = 0; tv.tv_sec = tv.tv_usec = 0;
STAILQ_FOREACH(sl, &shead, next) { STAILQ_FOREACH(sl, &shead, next) {
if (sl->sl_socket > fdsrmax) if (sl->sl_socket > fdsrmax)
fdsrmax = sl->sl_socket; fdsrmax = sl->sl_socket;
▲ Show 20 Lines • Show All 161 Lines • ▼ Show 20 Lines
/* /*
* Removes characters from log messages that are unsafe to display. * Removes characters from log messages that are unsafe to display.
* TODO: Permit UTF-8 strings that include a BOM per RFC 5424? * TODO: Permit UTF-8 strings that include a BOM per RFC 5424?
*/ */
static void static void
parsemsg_remove_unsafe_characters(const char *in, char *out, size_t outlen) parsemsg_remove_unsafe_characters(const char *in, char *out, size_t outlen)
{ {
char *q; const char *p, *end_p;
char *q, *end_q;
int c; int c;
size_t n, max_len;
p = in;
q = out; q = out;
while ((c = (unsigned char)*in++) != '\0' && q < out + outlen - 4) { end_p = p + strlen(p);
if (mask_C1 && (c & 0x80) && c < 0xA0) { end_q = out + outlen - 1;
markjUnsubmitted
Not Done
Inline Actions

mask_C1 is now a write-only variable. What does vis() do with C1 control characters?

markj: `mask_C1` is now a write-only variable. What does `vis()` do with C1 control characters?
yzhong_freebsdfoundation.orgAuthorUnsubmitted
Done
Inline Actions

From my testing, vis() automatically converts C1 characters. I added back the old behaviour for when mask_C1 is false.

yzhong_freebsdfoundation.org: From my testing, vis() automatically converts C1 characters. I added back the old behaviour for…
c &= 0x7F; while (*p != '\0' && q < end_q) {
markjUnsubmitted
Done
Inline Actions

end_q - q measures the number of bytes remaining in the output buffer, but we're passing the input buffer to mbrtoc32(). In particular, mbrtoc32() assumes that it can access up to n bytes of the input buffer, which is not always true.

max_len should be of type size_t.

markj: `end_q - q` measures the number of bytes remaining in the output buffer, but we're passing the…
markjUnsubmitted
Done
Inline Actions

It's better to write end_p = p + strlen(p) and check end_p - p rather than recomputing the entire string's length for each wide character.

markj: It's better to write `end_p = p + strlen(p)` and check `end_p - p` rather than recomputing the…
markjUnsubmitted
Done
Inline Actions

You could rewrite this as while (p < end_p && q < end_q) I think.

markj: You could rewrite this as `while (p < end_p && q < end_q)` I think.
*q++ = 'M'; max_len = MB_LEN_MAX > (end_p - p) ? (end_p - p) : MB_LEN_MAX;
*q++ = '-'; n = mbtowc(&c, p, max_len);
if (n == 0) {
break;
} else if (n == (size_t)-1 || n == (size_t)-2) {
/* Invalid char sequence for this charset. */
c = *p;
n = (size_t)1;
} }
if (isascii(c) && iscntrl(c)) {
if (c == '\n') { if (c == '\n') {
*q++ = ' '; *q++ = ' ';
} else if (c == '\t') { } else if (c == '\t') {
*q++ = '\t'; *q++ = '\t';
} else { } else if (!mask_C1 && (c & 0x80) && c < 0xA0) {
*q++ = '^'; *q++ = c;
*q++ = c ^ 0100; } else if (iswprint(c)) {
if (q > end_q - n) {
break;
} }
memcpy(q, p, n);
q += n;
markjUnsubmitted
Done
Inline Actions

Is the cast to size_t actually required?

Don't we need to limit that parameter to min(MB_LEN_MAX, <# bytes left>)?

markj: Is the cast to size_t actually required? Don't we need to limit that parameter to min…
} else { } else {
*q++ = c; if (q > end_q - 4) {
break;
markjUnsubmitted
Done
Inline Actions

When can mbrtoc32() return a value greater than n? It's not clear to me from reading the man page that that's permitted.

markj: When can mbrtoc32() return a value greater than `n`? It's not clear to me from reading the man…
} }
q = vis(q, c, 0, 0);
}
p += n;
} }
*q = '\0'; *q = '\0';
} }
/* /*
* Parses a syslog message according to RFC 5424, assuming that PRI and * Parses a syslog message according to RFC 5424, assuming that PRI and
* VERSION (i.e., "<%d>1 ") have already been parsed by parsemsg(). The * VERSION (i.e., "<%d>1 ") have already been parsed by parsemsg(). The
* parsed result is passed to logmsg(). * parsed result is passed to logmsg().
markjUnsubmitted
Done
Inline Actions

This might as well be a memcpy(). strncpy() doesn't guarantee nul-termination here, the assignment after the loop does.

markj: This might as well be a memcpy(). strncpy() doesn't guarantee nul-termination here, the…
*/ */
static void static void
parsemsg_rfc5424(const char *from, int pri, char *msg) parsemsg_rfc5424(const char *from, int pri, char *msg)
{ {
const struct logtime *timestamp; const struct logtime *timestamp;
struct logtime timestamp_remote; struct logtime timestamp_remote;
const char *omsg, *hostname, *app_name, *procid, *msgid, const char *omsg, *hostname, *app_name, *procid, *msgid,
*structured_data; *structured_data;
▲ Show 20 Lines • Show All 2,951 Lines • Show Last 20 Lines