From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/5430 Path: news.gmane.org!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: [PATCH] implement the LOG_PERROR option in syslog Date: Fri, 11 Jul 2014 00:49:50 -0400 Message-ID: <20140711044950.GY179@brightrain.aerifal.cx> References: <1404909732-83885-1-git-send-email-clement.vasseur@gmail.com> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Trace: ger.gmane.org 1405054214 10136 80.91.229.3 (11 Jul 2014 04:50:14 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Fri, 11 Jul 2014 04:50:14 +0000 (UTC) To: musl@lists.openwall.com Original-X-From: musl-return-5435-gllmg-musl=m.gmane.org@lists.openwall.com Fri Jul 11 06:50:04 2014 Return-path: Envelope-to: gllmg-musl@plane.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by plane.gmane.org with smtp (Exim 4.69) (envelope-from ) id 1X5SmZ-0002GE-RP for gllmg-musl@plane.gmane.org; Fri, 11 Jul 2014 06:50:03 +0200 Original-Received: (qmail 23862 invoked by uid 550); 11 Jul 2014 04:50:03 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: Original-Received: (qmail 23851 invoked from network); 11 Jul 2014 04:50:02 -0000 Content-Disposition: inline In-Reply-To: <1404909732-83885-1-git-send-email-clement.vasseur@gmail.com> User-Agent: Mutt/1.5.21 (2010-09-15) Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:5430 Archived-At: 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