<pre style='margin:0'>
Christopher Nielsen (mascguy) pushed a commit to branch master
in repository macports-legacy-support.
</pre>
<p><a href="https://github.com/macports/macports-legacy-support/commit/4b3dd3692b41aeb702ba73e95dd88eca8738a5de">https://github.com/macports/macports-legacy-support/commit/4b3dd3692b41aeb702ba73e95dd88eca8738a5de</a></p>
<pre style="white-space: pre; background: #F8F8F8"><span style='display:block; white-space:pre;color:#808000;'>commit 4b3dd3692b41aeb702ba73e95dd88eca8738a5de
</span>Author: Fred Wright <fw@fwright.net>
AuthorDate: Sat Sep 28 18:40:57 2024 -0700
<span style='display:block; white-space:pre;color:#404040;'> [v]dprintf(): Fix unwanted lock release.
</span><span style='display:block; white-space:pre;color:#404040;'>
</span><span style='display:block; white-space:pre;color:#404040;'> This uses a kludgy and nonportable method to avoid the ill effects of
</span><span style='display:block; white-space:pre;color:#404040;'> fclose() on locks (see the comment). It should be reliable for all
</span><span style='display:block; white-space:pre;color:#404040;'> relevant OSX versions, but it can be reverted if it causes trouble.
</span><span style='display:block; white-space:pre;color:#404040;'>
</span><span style='display:block; white-space:pre;color:#404040;'> TESTED:
</span><span style='display:block; white-space:pre;color:#404040;'> Passes tests, which would fail if the close() disable didn't work.
</span>---
src/dprintf.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 81 insertions(+), 9 deletions(-)
<span style='display:block; white-space:pre;color:#808080;'>diff --git a/src/dprintf.c b/src/dprintf.c
</span><span style='display:block; white-space:pre;color:#808080;'>index 4bdc47c..da7747a 100644
</span><span style='display:block; white-space:pre;background:#e0e0ff;'>--- a/src/dprintf.c
</span><span style='display:block; white-space:pre;background:#e0e0ff;'>+++ b/src/dprintf.c
</span><span style='display:block; white-space:pre;background:#e0e0e0;'>@@ -21,33 +21,105 @@
</span>
#include <errno.h>
#include <stdarg.h>
<span style='display:block; white-space:pre;background:#e0ffe0;'>+#include <stddef.h>
</span> #include <stdio.h>
#include <unistd.h>
<span style='display:block; white-space:pre;background:#e0ffe0;'>+/*
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * KLUDGE: Arrange to disable underlying close() in fclose().
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ *
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * If we simply use fdopen() and then fclose() to establish the temporary
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * stream, the fclose() issues a close() on the underlying fd, preventing
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * its further use. We can mostly avoid this (the earlier approach) by
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * using dup() to create a duplicate fd that can mostly be safely closed,
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * but since the duplicate fd shares any locks with the original, the close()
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * still releases the locks, possibly causing weird failures.
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ *
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * Apple's library code avoids this by creating a FILE as an auto variable
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * within the function, and appropriately initializing it to use the provided
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * fd. It finishes with an fflush() but not a close(), thereby leaving the
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * fd unperturbed except for the added data. But this approach relies on
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * knowledge of the private internal layout of the FILE, which is not
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * available to code outside the system libraries.
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ *
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * Without substantial internal knowledge, there's no way to create a FILE
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * without resorting to fdopen(). And since fdopen() uses a special
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * allocator, there's no way (again without private knowledge) to free
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * the resulting FILE other than with fclose().
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ *
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * Another approach which requires considerably less (but still nonzero)
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * internal knowledge is to set the internal '_close' function pointer to
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * NULL, which disables the close() while allowing the rest of fclose()
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * to operate normally (including the important fflush()).
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ *
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * The issue with this approach is knowing the offset for the _close element.
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * Fortunately, a few things work in our favor:
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * 1) The '_close' element is immediately preceded by a '_cookie' element,
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * which is actually a self pointer to the FILE itself. This allows
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * a fairly reliable consistency check.
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * 2) The layout of FILE (at least up through '_close') has never changed
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * across all the OS versions, avoiding the need for version conditions.
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * 3) This code is only applicable to OS versions 10.6 and earlier, which
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * have been frozen for ages, avoiding any future compatibility issues.
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ *
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * So what we do here is to define enough of the FILE layout to cover the
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * fields of interest (skipping the rest), use a check on the '_cookie'
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * element as a sanity check, and then (if it passes) set the '_close'
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * element to NULL. If the sanity check fails, we leave '_close' alone,
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * which reintroduces the unwanted close() of the fd, but that's a more
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * obvious failure than releasing locks (and is tested by the accompanying
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * tests).
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ *
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * Note that Apple didn't get around to hiding the private definitions
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * until the 11.x SDK, so we prefix our versions with '__mpls' to avoid
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ * conflicts.
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ */
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+/* stdio buffers */
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+struct __mpls__sbuf {
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ unsigned char *_base;
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ int _size;
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+};
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+/* stdio FILE object (truncated) */
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+struct __mpls__sFILE {
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ unsigned char *_p; /* current position in (some) buffer */
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ int _r; /* read space left for getc() */
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ int _w; /* write space left for putc() */
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ short _flags; /* flags, below; this FILE is free if 0 */
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ short _file; /* fileno, if Unix descriptor, else -1 */
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ struct __mpls__sbuf _bf; /* the buffer (at least 1 byte, if !NULL) */
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ int _lbfsize; /* 0 or -_bf._size, for inline putc */
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ /* operations */
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ void *_cookie; /* cookie passed to io functions */
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ int (*_close)(void *);
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ /* We don't need the rest */
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+};
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+
</span> int
vdprintf(int fildes, const char * __restrict format, va_list ap) {
FILE *stream;
<span style='display:block; white-space:pre;background:#e0ffe0;'>+ struct __mpls__sFILE *filep;
</span> int ret;
char buf[BUFSIZ];
<span style='display:block; white-space:pre;background:#ffe0e0;'>- /* Create a stream for a copy of the target fd, with our local buffer. */
</span><span style='display:block; white-space:pre;background:#ffe0e0;'>- stream = fdopen(dup(fildes), "w");
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ /* Create a stream for the target fd, with our local buffer. */
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ stream = fdopen(fildes, "w");
</span> if (stream == NULL) {
errno = EBADF; /* Set the expected errno if it fails */
return -1;
}
setbuffer(stream, buf, sizeof(buf));
<span style='display:block; white-space:pre;background:#e0ffe0;'>+ /* If the FILE looks as expected, clear the _close pointer. */
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ filep = (struct __mpls__sFILE *) stream;
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ if (filep->_cookie == filep) filep->_close = NULL;
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+
</span> /* Do the output. */
ret = vfprintf(stream, format, ap);
<span style='display:block; white-space:pre;background:#ffe0e0;'>- /*
</span><span style='display:block; white-space:pre;background:#ffe0e0;'>- * Close the FILE and the duplicate fd.
</span><span style='display:block; white-space:pre;background:#ffe0e0;'>- *
</span><span style='display:block; white-space:pre;background:#ffe0e0;'>- * NOTE: This releases any locks held by the original fd. There doesn't
</span><span style='display:block; white-space:pre;background:#ffe0e0;'>- * seem to be an easy way to avoid this, given that free(stream) doesn't
</span><span style='display:block; white-space:pre;background:#ffe0e0;'>- * work.
</span><span style='display:block; white-space:pre;background:#ffe0e0;'>- */
</span><span style='display:block; white-space:pre;background:#e0ffe0;'>+ /* Close the FILE (but not the fd). */
</span> if (fclose(stream)) ret = -1;
return ret;
</pre><pre style='margin:0'>
</pre>