[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20140711044950.GY179@brightrain.aerifal.cx>
Date: Fri, 11 Jul 2014 00:49:50 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] implement the LOG_PERROR option in syslog
On Wed, Jul 09, 2014 at 02:42:12PM +0200, Clément Vasseur wrote:
> Since musl defines this option in its header file, maybe it should
> actually do something.
> ---
> src/misc/syslog.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/src/misc/syslog.c b/src/misc/syslog.c
> index 57f1d75..2b0c73b 100644
> --- a/src/misc/syslog.c
> +++ b/src/misc/syslog.c
> @@ -79,7 +79,7 @@ static void _vsyslog(int priority, const char *message, va_list ap)
> char buf[256];
> int errno_save = errno;
> int pid;
> - int l, l2;
> + int l, l1, l2, l3;
>
> if (log_fd < 0) {
> __openlog();
> @@ -93,14 +93,16 @@ static void _vsyslog(int priority, const char *message, va_list ap)
> strftime(timebuf, sizeof timebuf, "%b %e %T", &tm);
>
> pid = (log_opt & LOG_PID) ? getpid() : 0;
> - l = snprintf(buf, sizeof buf, "<%d>%s %s%s%.0d%s: ",
> - priority, timebuf, log_ident, "["+!pid, pid, "]"+!pid);
> + l1 = snprintf(buf, sizeof buf, "<%d>%s ", priority, timebuf);
> + l2 = snprintf(buf+l1, sizeof buf - l1, "%s%s%.0d%s: ",
> + log_ident, "["+!pid, pid, "]"+!pid);
> errno = errno_save;
> - l2 = vsnprintf(buf+l, sizeof buf - l, message, ap);
> - if (l2 >= 0) {
> - if (l2 >= sizeof buf - l) l = sizeof buf - 1;
> - else l += l2;
> + l3 = vsnprintf(buf+l1+l2, sizeof buf - (l1+l2), message, ap);
> + if (l3 >= 0) {
> + if (l3 >= sizeof buf - (l1+l2)) l = sizeof buf - 1;
> + else l = l1+l2+l3;
> if (buf[l-1] != '\n') buf[l++] = '\n';
> + if (log_opt & LOG_PERROR) write(2, buf+l1, l-l1);
> send(log_fd, buf, l, 0);
> }
> }
This patch looks a lot more invasive than it needs to be, and (as
discussed off-list with Alexander's concerns that there might be a
buffer overflow here) makes it a lot less obvious that the code is
safe. The reason for the two separate snprintf/vsnprintf calls in the
original version of the file was purely because C does not allow
merging variadic argument lists. It's ugly as-is because it violates
the principle that snprintf should be used to construct the entire
string in one operation rather than in multiple calls with error-prone
arithmetic on the remaining space, but there's no way to avoid it.
Anyway, for adding the feature you want, where you just want to know
the number of initial bytes to skip writing to stderr, I would just
use %n to get the offset.
Rich
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
