From 1eac4f3880f10a4a9702939b60d322b40db08972 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko <vda.linux@googlemail.com> Date: Sun, 13 Dec 2009 04:00:52 +0100 Subject: syslog: fix openlog(xx, LOG_KERN) and optimize a bit The fix: logfac == 0 in openlog(xx, logfac) is allowed now. Corresponding internal openlog() call in vsyslog() uses explicit LOG_USER in order to set it as a default facility. Optimizations: mylock is not recursive now, since a single intenal call of openlog is converted to a call to openlog_internal which assumes that lock is already taken. No recursive locking is possible now. LogFacility is reduced to byte. cache static LogFile in auto variable fd (smaller code). vsyslog with bogus pri parameter wouldn't lock/unlock and mess with signals - it will just return at once. pass NULL as ident string in internal openlog call - same effect as passing LogTag but smaller code. comment out "if (LogTag)" checks - it is never NULL. use the same struct sigaction for setting new sigaction and for saving old one - saves ~32 bytes of stack. Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com> --- libc/misc/syslog/syslog.c | 102 ++++++++++++++++++++++++++-------------------- 1 file changed, 57 insertions(+), 45 deletions(-) (limited to 'libc/misc/syslog') diff --git a/libc/misc/syslog/syslog.c b/libc/misc/syslog/syslog.c index 794c0c17f..f66ba8faf 100644 --- a/libc/misc/syslog/syslog.c +++ b/libc/misc/syslog/syslog.c @@ -80,22 +80,21 @@ #include <signal.h> - #include <bits/uClibc_mutex.h> -__UCLIBC_MUTEX_STATIC(mylock, PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP); + +__UCLIBC_MUTEX_STATIC(mylock, PTHREAD_MUTEX_INITIALIZER); +static const char *LogTag = "syslog"; /* string to tag the entry with */ static int LogFile = -1; /* fd for log */ static smalluint connected; /* have done connect */ -/* all bits in option argument for openlog() fit in 8 bits */ -static smalluint LogStat = 0; /* status bits, set by openlog() */ -static const char *LogTag = "syslog"; /* string to tag the entry with */ -/* this fits in 8 bits too (LOG_LOCAL7 = 23<<3 = 184), - * but NB: LOG_FACMASK is bigger (= 0x03f8 = 127<<3) for some strange reason. - * Oh well. */ -static int LogFacility = LOG_USER;/* default facility code */ -/* bits mask of priorities (eight prios - 8 bits is enough) */ -static smalluint LogMask = 0xff; /* mask of priorities to be logged */ +/* all bits in option argument for openlog fit in 8 bits */ +static smalluint LogStat = 0; /* status bits, set by openlog */ +/* default facility code if openlog is not called */ +/* (this fits in 8 bits even without >> 3 shift, but playing extra safe) */ +static smalluint LogFacility = LOG_USER >> 3; +/* bits mask of priorities to be logged (eight prios - 8 bits is enough) */ +static smalluint LogMask = 0xff; /* AF_UNIX address of local logger (we use struct sockaddr * instead of struct sockaddr_un since "/dev/log" is small enough) */ static const struct sockaddr SyslogAddr = { @@ -115,45 +114,46 @@ closelog_intern(int sig) if (sig == 0) { /* called from closelog()? - reset to defaults */ LogStat = 0; LogTag = "syslog"; - LogFacility = LOG_USER; + LogFacility = LOG_USER >> 3; LogMask = 0xff; } } -/* - * OPENLOG -- open system log - */ -void -openlog(const char *ident, int logstat, int logfac) +static void +openlog_intern(const char *ident, int logstat, int logfac) { + int fd; int logType = SOCK_DGRAM; - __UCLIBC_MUTEX_LOCK(mylock); - if (ident != NULL) LogTag = ident; LogStat = logstat; - if (logfac != 0 && (logfac &~ LOG_FACMASK) == 0) - LogFacility = logfac; - if (LogFile == -1) { -retry: - if (LogStat & LOG_NDELAY) { - if ((LogFile = socket(AF_UNIX, logType, 0)) == -1) { - goto DONE; + /* (we were checking also for logfac != 0, but it breaks + * openlog(xx, LOG_KERN) since LOG_KERN == 0) */ + if ((logfac & ~LOG_FACMASK) == 0) /* if we don't have invalid bits */ + LogFacility = (unsigned)logfac >> 3; + + fd = LogFile; + if (fd == -1) { + retry: + if (logstat & LOG_NDELAY) { + LogFile = fd = socket(AF_UNIX, logType, 0); + if (fd == -1) { + return; } - fcntl(LogFile, F_SETFD, FD_CLOEXEC); + fcntl(fd, F_SETFD, FD_CLOEXEC); /* We don't want to block if e.g. syslogd is SIGSTOPed */ - fcntl(LogFile, F_SETFL, O_NONBLOCK | fcntl(LogFile, F_GETFL)); + fcntl(fd, F_SETFL, O_NONBLOCK | fcntl(fd, F_GETFL)); } } - if (LogFile != -1 && !connected) { - if (connect(LogFile, &SyslogAddr, sizeof(SyslogAddr)) != -1) { + if (fd != -1 && !connected) { + if (connect(fd, &SyslogAddr, sizeof(SyslogAddr)) != -1) { connected = 1; } else { - if (LogFile != -1) { - close(LogFile); - LogFile = -1; + if (fd != -1) { + close(fd); + LogFile = fd = -1; } if (logType == SOCK_DGRAM) { logType = SOCK_STREAM; @@ -161,8 +161,16 @@ retry: } } } +} -DONE: +/* + * OPENLOG -- open system log + */ +void +openlog(const char *ident, int logstat, int logfac) +{ + __UCLIBC_MUTEX_LOCK(mylock); + openlog_intern(ident, logstat, logfac); __UCLIBC_MUTEX_UNLOCK(mylock); } libc_hidden_def(openlog) @@ -180,25 +188,29 @@ vsyslog(int pri, const char *fmt, va_list ap) int fd, saved_errno; int rc; char tbuf[1024]; /* syslogd is unable to handle longer messages */ - struct sigaction action, oldaction; + struct sigaction action; + + /* Just throw out this message if pri has bad bits. */ + if ((pri & ~(LOG_PRIMASK|LOG_FACMASK)) != 0) + return; memset(&action, 0, sizeof(action)); action.sa_handler = closelog_intern; - sigaction(SIGPIPE, &action, &oldaction); + sigaction(SIGPIPE, &action, &action); saved_errno = errno; __UCLIBC_MUTEX_LOCK(mylock); - /* See if we should just throw out this message. */ - if (!(LogMask & LOG_MASK(LOG_PRI(pri))) || (pri &~ (LOG_PRIMASK|LOG_FACMASK))) + /* See if we should just throw out this message according to LogMask. */ + if ((LogMask & LOG_MASK(LOG_PRI(pri))) == 0) goto getout; if (LogFile < 0 || !connected) - openlog(LogTag, LogStat | LOG_NDELAY, 0); + openlog_intern(NULL, LogStat | LOG_NDELAY, LOG_USER); /* Set default facility if none specified. */ if ((pri & LOG_FACMASK) == 0) - pri |= LogFacility; + pri |= ((int)LogFacility << 3); /* Build the message. We know the starting part of the message can take * no longer than 64 characters plus length of the LogTag. So it's @@ -206,7 +218,7 @@ vsyslog(int pri, const char *fmt, va_list ap) */ (void)time(&now); stdp = p = tbuf + sprintf(tbuf, "<%d>%.15s ", pri, ctime(&now) + 4); - if (LogTag) { + /*if (LogTag) - always true */ { if (strlen(LogTag) < sizeof(tbuf) - 64) p += sprintf(p, "%s", LogTag); else @@ -214,7 +226,7 @@ vsyslog(int pri, const char *fmt, va_list ap) } if (LogStat & LOG_PID) p += sprintf(p, "[%d]", getpid()); - if (LogTag) { + /*if (LogTag) - always true */ { *p++ = ':'; *p++ = ' '; } @@ -253,7 +265,7 @@ vsyslog(int pri, const char *fmt, va_list ap) /* Output the message to the local logger using NUL as a message delimiter. */ p = tbuf; - *last_chr = 0; + *last_chr = '\0'; if (LogFile >= 0) { do { rc = write(LogFile, p, last_chr + 1 - p); @@ -288,9 +300,9 @@ vsyslog(int pri, const char *fmt, va_list ap) (void)close(fd); } -getout: + getout: __UCLIBC_MUTEX_UNLOCK(mylock); - sigaction(SIGPIPE, &oldaction, NULL); + sigaction(SIGPIPE, &action, NULL); } libc_hidden_def(vsyslog) -- cgit v1.2.3 From 6732cb1ae137d7af17eb911004ba904badba1b85 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko <vda.linux@googlemail.com> Date: Sun, 13 Dec 2009 05:47:19 +0100 Subject: syslog: use send(MSG_NOSIGNAL) instead of write, thus no need to handle SIGPIPE Size changes by this and previous change: text data bss dec hex filename 1151 13 2 1166 48e libc/misc/syslog/syslog.o 1093 10 2 1105 451 libc/misc/syslog/syslog.o 1047 10 2 1059 423 libc/misc/syslog/syslog.o Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com> --- libc/misc/syslog/syslog.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) (limited to 'libc/misc/syslog') diff --git a/libc/misc/syslog/syslog.c b/libc/misc/syslog/syslog.c index f66ba8faf..b10a55615 100644 --- a/libc/misc/syslog/syslog.c +++ b/libc/misc/syslog/syslog.c @@ -85,6 +85,10 @@ __UCLIBC_MUTEX_STATIC(mylock, PTHREAD_MUTEX_INITIALIZER); +/* !glibc_compat: glibc uses argv[0] by default + * (default: if there was no openlog or if openlog passed NULL), + * not string "syslog" + */ static const char *LogTag = "syslog"; /* string to tag the entry with */ static int LogFile = -1; /* fd for log */ static smalluint connected; /* have done connect */ @@ -188,16 +192,11 @@ vsyslog(int pri, const char *fmt, va_list ap) int fd, saved_errno; int rc; char tbuf[1024]; /* syslogd is unable to handle longer messages */ - struct sigaction action; /* Just throw out this message if pri has bad bits. */ if ((pri & ~(LOG_PRIMASK|LOG_FACMASK)) != 0) return; - memset(&action, 0, sizeof(action)); - action.sa_handler = closelog_intern; - sigaction(SIGPIPE, &action, &action); - saved_errno = errno; __UCLIBC_MUTEX_LOCK(mylock); @@ -268,7 +267,8 @@ vsyslog(int pri, const char *fmt, va_list ap) *last_chr = '\0'; if (LogFile >= 0) { do { - rc = write(LogFile, p, last_chr + 1 - p); + /* can't just use write, it can result in SIGPIPE */ + rc = send(LogFile, p, last_chr + 1 - p, MSG_NOSIGNAL); if (rc < 0) { /* I don't think looping forever on EAGAIN is a good idea. * Imagine that syslogd is SIGSTOPed... */ @@ -302,7 +302,6 @@ vsyslog(int pri, const char *fmt, va_list ap) getout: __UCLIBC_MUTEX_UNLOCK(mylock); - sigaction(SIGPIPE, &action, NULL); } libc_hidden_def(vsyslog) -- cgit v1.2.3