From d43551c9c9fa8bda04b32d50f364d2b40bf757cf Mon Sep 17 00:00:00 2001
From: Fred Wright <fw@fwright.net>
Date: Sat, 23 Nov 2024 13:30:18 -0800
Subject: [PATCH] fdopendir: Rework to handle variants directly in source.

This reworks the fdopendir code to directly build all the needed
variants, instead of the horrible splitandfilterandmergemultiarch
kludge in the Makefile.  Not only does it reduce code duplication, but
it also drastically improves readability.  It does not change the
underlying logic in the "guts", but merely changes the interfacing to
the different variants.

This was the only use of splitandfilterandmergemultiarch, which will
be excised in a subsequent commit.

A subtle problem that this introduced is that the empty object filter
for the static library was previously failing to filter fdopendir.o
due to the way it was constructed, but fixing that resulted in a truly
empty static library on 10.15+ platforms.  This is actually illegal,
so there's now a hack to put one dummy object into the archive in this
case.  The dummy object defines a single global symbol, thereby
avoiding the "has no symbols" warning from 'ar' and other tools.

TESTED:
Builds and passes all tests (including the new one in a subsequent
commit), on all platforms.
---
 Makefile         |  12 ++-
 src/dummylib.xxc |  13 +++
 src/fdopendir.c  | 217 +++++++++++++++++++++++++++++++++++++++--------
 3 files changed, 204 insertions(+), 38 deletions(-)
 create mode 100644 src/dummylib.xxc

diff --git a/Makefile b/Makefile
index 3890f57..12bae93 100644
--- a/Makefile
+++ b/Makefile
@@ -99,7 +99,7 @@ FIND_LIBHEADERS := find $(SRCINCDIR) -type f \( -name '*.h' -o \
 LIBHEADERS      := $(shell $(FIND_LIBHEADERS))
 ALLHEADERS      := $(LIBHEADERS) $(wildcard $(SRCDIR)/*.h)
 
-MULTISRCS       := $(SRCDIR)/fdopendir.c
+MULTISRCS       :=
 ADDSRCS         := $(SRCDIR)/add_symbols.c
 LIBSRCS         := $(filter-out $(MULTISRCS) $(ADDSRCS),$(wildcard $(SRCDIR)/*.c))
 
@@ -125,10 +125,16 @@ ALLSYSLIBOBJS   := $(ALLDLIBOBJS) $(ADDOBJS)
 # This not only reduces the size of the static library a bit, but also
 # avoids the "no symbols" warnings when creating it.
 #
+# A complication is that a completely empty static library is illegal,
+# so we provide a dummy object to be used when the library is logically
+# empty.
+#
 # This treatment is only applicable to the static library.
 EMPTY            = empty_source_content
 EMPTYSOBJ        = $(SRCDIR)/$(EMPTY)$(SLIBOBJEXT)
 SOBJLIST         = $(SRCDIR)/slibobjs.tmp
+DUMMYSRC         = $(SRCDIR)/dummylib.xxc
+DUMMYOBJ         = $(SRCDIR)/dummylib.o
 
 # Automatic tests that don't use the library, and are OK with -fno-builtin
 XTESTDIR          = xtest
@@ -337,9 +343,13 @@ slibobjs: $(ALLSLIBOBJS)
 allobjs: dlibobjs slibobjs syslibobjs
 
 # Create a list of nonempty static object files.
+# Since completely empty archives are illegal, we use our dummy if there
+# would otherwise be no objects.
 $(SOBJLIST): $(ALLSLIBOBJS)
 	$(CC) -c $(ALLCFLAGS) $(SLIBCFLAGS) -xc /dev/null -o $(EMPTYSOBJ)
+	$(CC) -c $(ALLCFLAGS) $(SLIBCFLAGS) -xc $(DUMMYSRC) -o $(DUMMYOBJ)
 	for f in $^; do cmp -s $(EMPTYSOBJ) $$f || echo $$f; done > $@
+	if [ ! -s $@ ]; then echo $(DUMMYOBJ) > $@; fi
 
 # Make the directories separate targets to avoid collisions in parallel builds.
 $(BUILDLIBDIR) $(DESTDIR)$(LIBDIR):
diff --git a/src/dummylib.xxc b/src/dummylib.xxc
new file mode 100644
index 0000000..5172d75
--- /dev/null
+++ b/src/dummylib.xxc
@@ -0,0 +1,13 @@
+/* dummylib - mostly empty item containing one global definition. */
+/*
+ * When the legacy-support library becomes logically empty, it still needs
+ * to contain at least one object to keep the tools happy.  It also needs
+ * to define at least one global to avoid "no symbols" warnings.  This
+ * source is used to provide such an object.  Currently this happens on
+ * 10.15+.
+ *
+ * This source is actually C, but uses a different extension to defend
+ * against wildcarding in the Makefile.
+ */
+
+int __LEGACY_SUPPORT_LIBRARY_IS_INTENTIONALLY_LEFT_BLANK__ = 0;
diff --git a/src/fdopendir.c b/src/fdopendir.c
index 3af64b0..f11d9c5 100644
--- a/src/fdopendir.c
+++ b/src/fdopendir.c
@@ -21,66 +21,209 @@
 /* Do our SDK-related setup */
 #include <_macports_extras/sdkversion.h>
 
+/*
+ * Implementation behavior largely follows these man page descriptions:
+ *
+ * https://www.freebsd.org/cgi/man.cgi?query=fdopendir&sektion=3
+ * https://linux.die.net/man/3/fdopendir
+ */
+
 #if __MPLS_LIB_SUPPORT_FDOPENDIR__
 
-#include "common-priv.h"
+/*
+ * Set up to use ino32 variants where possible.  This results in generating
+ * the "unadorned" names, which we augment with explicit suffixes where
+ * needed.  Similarly, we use the non-POSIX form in 32-bit builds.
+ *
+ * The ino32 variants are known to be unavailable in arm64 builds, but are
+ * available in all OS versions that need our fdopendir().
+ *
+ * Although the struct dirent is formatted differently for ino32 and ino64,
+ * we never directly reference it here.  The only difference in DIR is the
+ * format of the private struct _telldir pointed to by the __dd_td field,
+ * which we also never reference here.  Hence, we don't care which variant
+ * we're using, except for passing the choice through to the underlying
+ * functions.
+ *
+ * In the case of struct stat, we provide our own ino64 variant where needed.
+ */
+
+#define _DARWIN_NO_64_BIT_INODE 1
+#if !__MPLS_64BIT
+#define _NONSTD_SOURCE
+#endif
 
 #include <dirent.h>
+#include <stddef.h>
+
 #include <sys/stat.h>
 
+#include "common-priv.h"
+
 #if __MPLS_SDK_MAJOR < 1050
 #define __dd_fd dd_fd
-#endif /* __MPLS_SDK_MAJOR < 1050 */
+#endif /* __MPLS_SDK_MAJOR < 1050
+*/
+
+/* Define an ino64 struct stat if possible, else fall back to standard. */
+#ifdef __DARWIN_STRUCT_STAT64
+  struct local_stat64 __DARWIN_STRUCT_STAT64;
+  typedef struct local_stat64 local_stat64_t;
+#else
+  typedef struct stat local_stat64_t;
+#endif
+
+/* Universal stat buffer, accommodating both formats */
+union stat_u {
+  struct stat s;
+  local_stat64_t s64;
+};
+
+/* Type declarations for external functions */
+typedef int (stat_fn_t)(int fd, struct stat *buf);
+typedef int (stat64_fn_t)(int fd, local_stat64_t *buf);
+typedef DIR * (opn_fn_t)(const char *dirname);
+typedef void (rwd_fn_t)(DIR *dirp);
+
+/* Structure for per-variant dispatch table */
+typedef struct funcs_s {
+  stat_fn_t *do_fstat;
+  stat64_fn_t *do_fstat64;
+  opn_fn_t *do_open;
+  rwd_fn_t *do_rewind;
+} funcs_t;
+
+/* Common function used by all variants - controlled by a dispatch table */
+static DIR *
+fdopendir_internal(int fd, const funcs_t *funcs) {
+  DIR *dir;
+  int err;
+  mode_t mode;
+  union stat_u stbuf;
+
+  /* Do the appropriate fstat() on the supplied fd */
+  if (funcs->do_fstat64) {
+    err = (*funcs->do_fstat64)(fd, &stbuf.s64);
+    mode = stbuf.s64.st_mode;
+  } else if (funcs->do_fstat) {
+    err = (*funcs->do_fstat)(fd, &stbuf.s);
+    mode = stbuf.s.st_mode;
+  } else {
+    errno = EINVAL;  /* Should be impossible */
+    return NULL;
+  }
+
+  /* Fail if fd isn't a valid open fd */
+  if (err < 0) {
+    return NULL;
+  }
+
+  /* Fail if fd isn't a directory */
+  if (!S_ISDIR(mode)) {
+    errno = ENOTDIR;
+    return NULL;
+  }
+
+  /* Open given directory fd safely for iteration via readdir */
+
+  dir = _ATCALL(fd, ".", NULL, (*funcs->do_open)("."));
+  if (!dir) {
+    return NULL;
+  }
+
+  /*
+   * Replace underlying fd with supplied fd
+   * A subsequent closedir() will close fd
+   */
+
+  (void)close(dir->__dd_fd);
+  dir->__dd_fd = fd;
+
+  /*
+   * Rewind to the start of the directory, in case the underlying file
+   * is not positioned at the start
+   */
+
+  (*funcs->do_rewind)(dir);
+
+  /* Close given fd on exec (as per fdopendir() docs) */
+
+  (void)fcntl(fd, F_SETFD, FD_CLOEXEC);
+
+  return dir;
+}
 
 /*
- * Implementation behavior largely follows these man page descriptions:
+ * Now to handle all the variants:
  *
- * https://www.freebsd.org/cgi/man.cgi?query=fdopendir&sektion=3
- * https://linux.die.net/man/3/fdopendir
+ * Define a macro listing all variants supported by the OS/arch.
+ * 10.4 lacks the INODE64 variants.
+ * 64-bit builds lack the UNIX2003 variants.
  */
 
-DIR *fdopendir(int dirfd) {
-    DIR *dir;
-    struct stat dirstat;
+#if __MPLS_TARGET_OSVER < 1050
+#if !__MPLS_64BIT
 
-    /* Fail if dirfd isn't a valid open fd */
-    if (fstat(dirfd, &dirstat) < 0) {
-        return NULL;
-    }
+#define ALL_VARIANTS \
+  VARIANT_ENT(basic,,,) \
+  VARIANT_ENT(posix,,$UNIX2003,)
 
-    /* Fail if dirfd isn't a directory */
-    if (!S_ISDIR(dirstat.st_mode)) {
-        errno = ENOTDIR;
-        return NULL;
-    }
+#else /* __MPLS_64BIT */
 
-    /* Open given directory fd safely for iteration via readdir */
+#define ALL_VARIANTS \
+  VARIANT_ENT(basic,,,)
 
-    dir = _ATCALL(dirfd, ".", NULL, opendir("."));
-    if (!dir) {
-        return NULL;
-    }
+#endif /* __MPLS_64BIT */
+#else /* __MPLS_TARGET_OSVER >= 1050 */
+#if !__MPLS_64BIT
 
-    /*
-     * Replace underlying fd with supplied dirfd
-     * A subsequent closedir() will close dirfd
-     */
+#define ALL_VARIANTS \
+  VARIANT_ENT(basic,,,) \
+  VARIANT_ENT(posix,,$UNIX2003,) \
+  VARIANT_ENT(ino64,$INODE64,$UNIX2003,64)
 
-    (void)close(dir->__dd_fd);
-    dir->__dd_fd = dirfd;
+#else /* __MPLS_64BIT */
 
-    /*
-     * Rewind to the start of the directory, in case the underlying file
-     * is not positioned at the start
-     */
+#define ALL_VARIANTS \
+  VARIANT_ENT(basic,,,) \
+  VARIANT_ENT(ino64,$INODE64,,64)
 
-    rewinddir(dir);
+#endif /* __MPLS_64BIT */
+#endif /* __MPLS_TARGET_OSVER >= 1050 */
 
-    /* Close given fd on exec (as per fdopendir() docs) */
+/* Declare all called functions with appropriate suffixes. */
+/* The "basic" case is redundant but serves as an error check. */
+#define VARIANT_ENT(name,isfx,usfx,is64) \
+stat##is64##_fn_t fstat##isfx; \
+opn_fn_t opendir##isfx##usfx; \
+rwd_fn_t rewinddir##isfx##usfx;
+ALL_VARIANTS
+#undef VARIANT_ENT
 
-    (void)fcntl(dirfd, F_SETFD, FD_CLOEXEC);
-
-    return dir;
+/* Generate dispatch tables. */
+/*
+ * Since compilers aren't smart enough to avoid complaining about mismatched
+ * types from values made irrelevant by compile-time constants, we include
+ * technically incorrect casts to shut them up.
+ */
+#define VARIANT_ENT(name,isfx,usfx,is64) \
+static const funcs_t name = { \
+  .do_fstat = 0##is64 ? NULL : (stat_fn_t *) &fstat##isfx, \
+  .do_fstat64 = 0##is64 ? (stat64_fn_t *) &fstat##isfx : NULL, \
+  .do_open = &opendir##isfx##usfx, \
+  .do_rewind = &rewinddir##isfx##usfx, \
+};
+ALL_VARIANTS
+#undef VARIANT_ENT
+
+/* Now generate the functions. */
+#define VARIANT_ENT(name,isfx,usfx,is64) \
+DIR * \
+fdopendir##isfx##usfx(int fd) \
+{ \
+  return fdopendir_internal(fd, &name); \
 }
+ALL_VARIANTS
+#undef VARIANT_ENT
 
 #endif /* __MPLS_LIB_SUPPORT_FDOPENDIR__ */
-- 
GitLab