From 3016e67c21c8ea1f1c44528c7895fad1761406c3 Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Tue, 16 Jan 2024 10:35:16 -0500
Subject: [PATCH] `bind`: give same treatment as `connect` in #8544, dedup

It is good to propagate the underlying error so whether or not we use a
process to deal with path length issues is not observable.

Also, as these wrapper functions got more and more complex, the code
duplication got worse and worse. The new `bindConnectProcHelper`
function deduplicates them.
---
 src/libutil/unix-domain-socket.cc | 84 ++++++++++++-------------------
 1 file changed, 33 insertions(+), 51 deletions(-)

diff --git a/src/libutil/unix-domain-socket.cc b/src/libutil/unix-domain-socket.cc
index 3b6d54a2c..0bcf9040d 100644
--- a/src/libutil/unix-domain-socket.cc
+++ b/src/libutil/unix-domain-socket.cc
@@ -38,52 +38,20 @@ AutoCloseFD createUnixDomainSocket(const Path & path, mode_t mode)
     return fdSocket;
 }
 
-static struct sockaddr* safeSockAddrPointerCast(struct sockaddr_un *addr) {
-    // Casting between types like these legacy C library interfaces require
-    // is forbidden in C++.
-    // To maintain backwards compatibility, the implementation of the
-    // bind function contains some hints to the compiler that allow for this
+
+static void bindConnectProcHelper(
+    std::string_view operationName, auto && operation,
+    int fd, const std::string & path)
+{
+    struct sockaddr_un addr;
+    addr.sun_family = AF_UNIX;
+
+    // Casting between types like these legacy C library interfaces
+    // require is forbidden in C++. To maintain backwards
+    // compatibility, the implementation of the bind/connect functions
+    // contains some hints to the compiler that allow for this
     // special case.
-    return reinterpret_cast<struct sockaddr *>(addr);
-}
-
-void bind(int fd, const std::string & path)
-{
-    unlink(path.c_str());
-
-    struct sockaddr_un addr;
-    addr.sun_family = AF_UNIX;
-    auto psaddr {safeSockAddrPointerCast(&addr)};
-
-    if (path.size() + 1 >= sizeof(addr.sun_path)) {
-        Pid pid = startProcess([&] {
-            Path dir = dirOf(path);
-            if (chdir(dir.c_str()) == -1)
-                throw SysError("chdir to '%s' failed", dir);
-            std::string base(baseNameOf(path));
-            if (base.size() + 1 >= sizeof(addr.sun_path))
-                throw Error("socket path '%s' is too long", base);
-            memcpy(addr.sun_path, base.c_str(), base.size() + 1);
-            if (bind(fd, psaddr, sizeof(addr)) == -1)
-                throw SysError("cannot bind to socket '%s'", path);
-            _exit(0);
-        });
-        int status = pid.wait();
-        if (status != 0)
-            throw Error("cannot bind to socket '%s'", path);
-    } else {
-        memcpy(addr.sun_path, path.c_str(), path.size() + 1);
-        if (bind(fd, psaddr, sizeof(addr)) == -1)
-            throw SysError("cannot bind to socket '%s'", path);
-    }
-}
-
-
-void connect(int fd, const std::string & path)
-{
-    struct sockaddr_un addr;
-    addr.sun_family = AF_UNIX;
-    auto psaddr {safeSockAddrPointerCast(&addr)};
+    auto * psaddr = reinterpret_cast<struct sockaddr *>(&addr);
 
     if (path.size() + 1 >= sizeof(addr.sun_path)) {
         Pipe pipe;
@@ -98,8 +66,8 @@ void connect(int fd, const std::string & path)
                 if (base.size() + 1 >= sizeof(addr.sun_path))
                     throw Error("socket path '%s' is too long", base);
                 memcpy(addr.sun_path, base.c_str(), base.size() + 1);
-                if (connect(fd, psaddr, sizeof(addr)) == -1)
-                    throw SysError("cannot connect to socket at '%s'", path);
+                if (operation(fd, psaddr, sizeof(addr)) == -1)
+                    throw SysError("cannot %s to socket at '%s'", operationName, path);
                 writeFull(pipe.writeSide.get(), "0\n");
             } catch (SysError & e) {
                 writeFull(pipe.writeSide.get(), fmt("%d\n", e.errNo));
@@ -110,16 +78,30 @@ void connect(int fd, const std::string & path)
         pipe.writeSide.close();
         auto errNo = string2Int<int>(chomp(drainFD(pipe.readSide.get())));
         if (!errNo || *errNo == -1)
-            throw Error("cannot connect to socket at '%s'", path);
+            throw Error("cannot %s to socket at '%s'", operationName, path);
         else if (*errNo > 0) {
             errno = *errNo;
-            throw SysError("cannot connect to socket at '%s'", path);
+            throw SysError("cannot %s to socket at '%s'", operationName, path);
         }
     } else {
         memcpy(addr.sun_path, path.c_str(), path.size() + 1);
-        if (connect(fd, psaddr, sizeof(addr)) == -1)
-            throw SysError("cannot connect to socket at '%s'", path);
+        if (operation(fd, psaddr, sizeof(addr)) == -1)
+            throw SysError("cannot %s to socket at '%s'", operationName, path);
     }
 }
 
+
+void bind(int fd, const std::string & path)
+{
+    unlink(path.c_str());
+
+    bindConnectProcHelper("bind", ::bind, fd, path);
+}
+
+
+void connect(int fd, const std::string & path)
+{
+    bindConnectProcHelper("connect", ::connect, fd, path);
+}
+
 }