From 3016e67c21c8ea1f1c44528c7895fad1761406c3 Mon Sep 17 00:00:00 2001 From: John Ericson 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(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(&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(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); +} + }