From patchwork Fri Feb 9 15:26:01 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 85533 X-Patchwork-Delegate: azanella@linux.vnet.ibm.com Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 1601B38582B9 for ; Fri, 9 Feb 2024 15:27:48 +0000 (GMT) X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 2AA3638582A3 for ; Fri, 9 Feb 2024 15:26:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2AA3638582A3 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 2AA3638582A3 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1707492369; cv=none; b=oD/GWxO3yY/ZT96VaqbE4dDFdJ84/82JnFXfYzAbDUivtbJmNbJF/gdTBUzBwqRtsm2fGRq2NqXnbSZrzgrlXl5ZRB2n0Yfd9UHS+zJGj0mJfV8qTnj6RYsGl5Q0Tyn09kAXgXfyymxOGuqLKStyAYZdH3SVnwj8nbwe0gicRNE= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1707492369; c=relaxed/simple; bh=Z21CqlVFxRC2yZpaWXikI1+LZg8FkpQfQZCWsqbVPlI=; h=DKIM-Signature:From:To:Subject:Message-ID:Date:MIME-Version; b=xyLdAmJwY6SfrFpns2TvhgP+R9AhHIrTc+3YR3a6puNsQVMX+j1JgSmCmJHM73HGDxUGw6/Bf5ud/+rBaYYSTbOmC283p7P05QSz6jvs/rFkULUAyFng0qb7sr6AepzUTGkY05JVTyuAfRUtR5XqrlK/3yjbzfmgGyPTiervskg= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1707492366; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=RnsV1jBuQmpN+F4yvTFeZy1CVilvDazUE3/mSf8AI5A=; b=DGmFVHGprpk3GTHq2xHlzFa2ANTLwV5aBtwQQUdy0xIUYda3/nbOz7BpKpFdWLJ0eUiMec nYQ5xVVES6O3OG3nGr0leKRNAW2TjrbJRdCPDoCgNwvV98YDsIF1FKiyaM+xZmS62MSL4W IjxQvo1zaWlVy12goBLzQmoozUVk55A= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-692-Q-_xw3UAMLKBg21Kj3RZKg-1; Fri, 09 Feb 2024 10:26:04 -0500 X-MC-Unique: Q-_xw3UAMLKBg21Kj3RZKg-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 4BF5428B6A2C for ; Fri, 9 Feb 2024 15:26:04 +0000 (UTC) Received: from oldenburg.str.redhat.com (unknown [10.39.192.58]) by smtp.corp.redhat.com (Postfix) with ESMTPS id AB52F492BC6 for ; Fri, 9 Feb 2024 15:26:02 +0000 (UTC) From: Florian Weimer To: libc-alpha@sourceware.org Subject: [PATCH 11/11] syslog: Use a printf buffer directly to construct the entire packet In-Reply-To: Message-ID: References: X-From-Line: a709cb1c36c0e92954ebb036adabc350428ee7d5 Mon Sep 17 00:00:00 2001 Date: Fri, 09 Feb 2024 16:26:01 +0100 User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.3 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.9 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.0 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H5, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libc-alpha-bounces+patchwork=sourceware.org@sourceware.org This defers buffer management largely to the asprintf implementation. It is quite close to the original implementation around open_memstream, except that an on-stack buffer is used for shorter messages, and that strftime no longer writes directly into the buffer. The new version no longer uses the (slow) %n format specifier. It also fixes an issue in the localtime_r failure path, where the message is prefixed with ": " due to an incorrect placement of the %n specifier. --- misc/syslog.c | 184 ++++++++++++++++---------------------------------- 1 file changed, 58 insertions(+), 126 deletions(-) diff --git a/misc/syslog.c b/misc/syslog.c index 68ee3aef5f..50cb252f47 100644 --- a/misc/syslog.c +++ b/misc/syslog.c @@ -38,13 +38,11 @@ static char sccsid[] = "@(#)syslog.c 8.4 (Berkeley) 3/18/94"; #include #include #include -#include -#include #include -#include #include #include #include +#include static int LogType = SOCK_DGRAM; /* type of socket connection */ static int LogFile = -1; /* fd for log */ @@ -60,27 +58,23 @@ __libc_lock_define_initialized (static, syslog_lock) static void openlog_internal (const char *, int, int); static void closelog_internal (void); -struct cleanup_arg +/* Unlock and deallocate the buffer. */ +static void +cancel_handler_buf (void *ptr) { - void *buf; - struct sigaction *oldaction; -}; + __libc_lock_unlock (syslog_lock); + + struct __printf_buffer_asprintf *buf = ptr; + __printf_buffer_asprintf_free (buf); +} +/* Only unlock. */ static void cancel_handler (void *ptr) { - /* Restore the old signal handler. */ - struct cleanup_arg *clarg = (struct cleanup_arg *) ptr; - - if (clarg != NULL) - /* Free the memstream buffer, */ - free (clarg->buf); - - /* Free the lock. */ __libc_lock_unlock (syslog_lock); } - /* * syslog, vsyslog -- * print message on log file; output is intended for syslogd(8). @@ -122,16 +116,21 @@ __vsyslog_chk (int pri, int flag, const char *fmt, va_list ap) __vsyslog_internal (pri, fmt, ap, (flag > 0) ? PRINTF_FORTIFY : 0); } +/* Send the buffer contents over the syslog socket. */ +static ssize_t +__syslog_send (struct __printf_buffer *buf) +{ + /* Send the trailing NUL byte for LogType == SOCK_STREAM only. */ + return __send (LogFile, buf->write_base, + buf->write_ptr - buf->write_base - (LogType != SOCK_STREAM), + MSG_NOSIGNAL); +} + void __vsyslog_internal (int pri, const char *fmt, va_list ap, unsigned int mode_flags) { - /* Try to use a static buffer as an optimization. */ - char bufs[1024]; - char *buf = bufs; - size_t bufsize; - - int msgoff; + ptrdiff_t msgoff; int saved_errno = errno; #define INTERNALLOG LOG_ERR|LOG_CONS|LOG_PERROR|LOG_PID @@ -144,8 +143,9 @@ __vsyslog_internal (int pri, const char *fmt, va_list ap, /* Prepare for multiple users. We have to take care: most syscalls we are using are cancellation points. */ - struct cleanup_arg clarg = { NULL, NULL }; - __libc_cleanup_push (cancel_handler, &clarg); + struct __printf_buffer_asprintf buf; + __printf_buffer_asprintf_init (&buf); + __libc_cleanup_push (cancel_handler_buf, &buf); __libc_lock_lock (syslog_lock); /* Check priority against setlogmask values. */ @@ -173,122 +173,56 @@ __vsyslog_internal (int pri, const char *fmt, va_list ap, __strftime_l (timestamp, sizeof timestamp, "%h %e %T ", now_tmp, _nl_C_locobj_ptr); -#define SYSLOG_HEADER(__pri, __timestamp, __msgoff, pid) \ - "<%d>%s%n%s%s%.0d%s: ", \ - __pri, __timestamp, __msgoff, \ - LogTag == NULL ? __progname : LogTag, \ - "[" + (pid == 0), pid, "]" + (pid == 0) + __printf_buffer (&buf.base, "<%d>", pri); -#define SYSLOG_HEADER_WITHOUT_TS(__pri, __msgoff) \ - "<%d>: %n", __pri, __msgoff - - int l, vl; if (has_ts) - l = __snprintf (bufs, sizeof bufs, - SYSLOG_HEADER (pri, timestamp, &msgoff, pid)); - else - l = __snprintf (bufs, sizeof bufs, - SYSLOG_HEADER_WITHOUT_TS (pri, &msgoff)); - if (l < 0) - goto out; - - char *pos; - size_t len; - - if (l < sizeof bufs) { - /* At this point, there is still a chance that we can print the - remaining part of the log into bufs and use that. */ - pos = bufs + l; - len = sizeof (bufs) - l; + __printf_buffer_puts (&buf.base, timestamp); + msgoff = buf.base.write_ptr - buf.base.write_base; + __printf_buffer_puts (&buf.base, LogTag == NULL ? __progname : LogTag); + if (pid != 0) + __printf_buffer (&buf.base, "[%d]", pid); + __printf_buffer_putc (&buf.base, ':'); + __printf_buffer_putc (&buf.base, ' '); } else { - buf = NULL; - /* We already know that bufs is too small to use for this log message. - The next vsnprintf into bufs is used only to calculate the total - required buffer length. We will discard bufs contents and allocate - an appropriately sized buffer later instead. */ - pos = bufs; - len = sizeof (bufs); + msgoff = buf.base.write_ptr - buf.base.write_base; + __printf_buffer_putc (&buf.base, ':'); + __printf_buffer_putc (&buf.base, ' '); } - { - va_list apc; - va_copy (apc, ap); - - /* Restore errno for %m format. */ - __set_errno (saved_errno); - - vl = __vsnprintf_internal (pos, len, fmt, apc, mode_flags); - va_end (apc); + /* Do not count the bytes written to the buffer so far. This value + is negative and accounts for the existing buffer contents, which + is not upposed to be visible to the caller. */ + buf.base.written = buf.base.write_base - buf.base.write_ptr; - if (vl < 0 || vl >= INT_MAX - l) - goto out; + errno = saved_errno; + __vprintf_buffer (&buf.base, fmt, ap, mode_flags); - if (vl >= len) - buf = NULL; - - bufsize = l + vl; - } - - if (buf == NULL) - { - buf = malloc ((bufsize + 1) * sizeof (char)); - if (buf != NULL) - { - /* Tell the cancellation handler to free this buffer. */ - clarg.buf = buf; + /* We may need the terminator if we are sending over a SOCK_STREAM + connection. Add this unconditionally to simplify error handling. */ + __printf_buffer_putc (&buf.base, '\0'); - int cl; - if (has_ts) - cl = __snprintf (buf, l + 1, - SYSLOG_HEADER (pri, timestamp, &msgoff, pid)); - else - cl = __snprintf (buf, l + 1, - SYSLOG_HEADER_WITHOUT_TS (pri, &msgoff)); - if (cl != l) - goto out; - - va_list apc; - va_copy (apc, ap); - cl = __vsnprintf_internal (buf + l, bufsize - l + 1, fmt, apc, - mode_flags); - va_end (apc); - - if (cl != vl) - goto out; - } - else - { - int bl; - /* Nothing much to do but emit an error message. */ - bl = __snprintf (bufs, sizeof bufs, - "out of memory[%d]", __getpid ()); - if (bl < 0 || bl >= sizeof bufs) - goto out; - - bufsize = bl; - buf = bufs; - msgoff = 0; - } - } + if (__printf_buffer_has_failed (&buf.base)) + goto out; /* Output to stderr if requested. */ if (LogStat & LOG_PERROR) - __dprintf (STDERR_FILENO, "%s%s", buf + msgoff, - "\n" + (buf[bufsize - 1] == '\n')); + { + /* The buffer always starts with '<' and a digit, so there are + at least two bytes, and -2 is valid offset from the next + to-be-written byte. -2 accounts for the NUL byte. */ + int has_nl = buf.base.write_ptr[-2] == '\n'; + __dprintf (STDERR_FILENO, "%s%s", buf.base.write_base + msgoff, + "\n" + has_nl); + } /* Get connected, output the message to the local logger. */ if (!connected) openlog_internal (NULL, LogStat | LOG_NDELAY, LogFacility); - /* If we have a SOCK_STREAM connection, also send ASCII NUL as a record - terminator. */ - if (LogType == SOCK_STREAM) - ++bufsize; - - if (!connected || __send (LogFile, buf, bufsize, MSG_NOSIGNAL) < 0) + if (!connected || __syslog_send (&buf.base) < 0) { if (connected) { @@ -297,7 +231,7 @@ __vsyslog_internal (int pri, const char *fmt, va_list ap, openlog_internal (NULL, LogStat | LOG_NDELAY, LogFacility); } - if (!connected || __send (LogFile, buf, bufsize, MSG_NOSIGNAL) < 0) + if (!connected || __syslog_send (&buf.base) < 0) { closelog_internal (); /* attempt re-open next time */ /* @@ -311,7 +245,7 @@ __vsyslog_internal (int pri, const char *fmt, va_list ap, (fd = __open (_PATH_CONSOLE, O_WRONLY | O_NOCTTY | O_CLOEXEC, 0)) >= 0) { - __dprintf (fd, "%s\r\n", buf + msgoff); + __dprintf (fd, "%s\r\n", buf.base.write_base + msgoff); __close (fd); } } @@ -321,9 +255,7 @@ out: /* End of critical section. */ __libc_cleanup_pop (0); __libc_lock_unlock (syslog_lock); - - if (buf != bufs) - free (buf); + __printf_buffer_asprintf_free (&buf); } /* AF_UNIX address of local logger */