review pass: fix the eaten Tab, drop the duplicated code, cover the gaps

1 files changed, 41 insertions, 18 deletions

diff --git a/src/nested.zig b/src/nested.zig
index 6e70bdc9..0c065d1d 100644
--- a/src/nested.zig
+++ b/src/nested.zig
@@ -17,15 +17,11 @@
 //! socket sits at a path anyone can derive from a pid — `Exec …` arriving here
 //! is not something this protocol is allowed to say.
 //!
-//! Linux only. ponytail: darwin has no /proc, so the walk there is
-//! proc_pidinfo(PROC_PIDTBSDINFO) for `pbi_ppid` plus a `pbi_comm` compare —
-//! a 16-byte truncated name, which is a weaker identity than an exe path —
-//! and neither SOCK_CLOEXEC nor accept4 exists, so the socket half needs two
-//! extra fcntl(FD_CLOEXEC) calls. Its `sockaddr.un.path` is 104 bytes, not
-//! 108: the `[108]u8` buffers and the unguarded memcpys below are sized for
-//! linux and a port has to re-derive them from `@FieldType`. None of it is
-//! testable from here, so detection is simply off: a pardes inside a pardes on
-//! macOS opens a second session the way it always did.
+//! Linux only. ponytail: darwin has no /proc, no SOCK_CLOEXEC and no accept4,
+//! and its `sockaddr.un.path` is 104 bytes rather than the 108 every buffer
+//! and unguarded memcpy below assumes. None of that is testable from here, so
+//! detection is simply off: a pardes inside a pardes on macOS opens a second
+//! session the way it always did.
 const std = @import("std");
 const builtin = @import("builtin");
 const libc = std.c;
@@ -69,7 +65,7 @@ pub fn socketPath(buf: *[108]u8, pid: libc.pid_t) ?[:0]const u8 {
 /// link reads `/path/to/pardes (deleted)` while the freshly built child's
 /// reads `/path/to/pardes`. Comparing them raw made every nested launch after
 /// a rebuild open a second full-screen UI inside the pane.
-pub fn stripDeleted(link: []const u8) []const u8 {
+fn stripDeleted(link: []const u8) []const u8 {
     const suffix = " (deleted)";
     return if (std.mem.endsWith(u8, link, suffix)) link[0 .. link.len - suffix.len] else link;
 }
@@ -78,7 +74,7 @@ pub fn stripDeleted(link: []const u8) []const u8 {
 /// /proc/<pid>/stat: that field is positional after `comm`, and a comm may
 /// contain spaces and parentheses — a process named `sh (a b)` shifts every
 /// field after it and the parse silently reads the wrong number.
-pub fn parsePPid(status: []const u8) ?libc.pid_t {
+fn parsePPid(status: []const u8) ?libc.pid_t {
     var lines = std.mem.splitScalar(u8, status, '\n');
     while (lines.next()) |line| {
         if (!std.mem.startsWith(u8, line, "PPid:")) continue;
@@ -90,7 +86,7 @@ pub fn parsePPid(status: []const u8) ?libc.pid_t {
 /// The pid in a `pardes-<pid>.sock` filename, for the startup sweep. Strictly
 /// digits: parseInt alone would take `pardes-+7.sock` and `pardes--7.sock`,
 /// and the sweep unlinks what this answers about.
-pub fn sweepPid(name: []const u8) ?libc.pid_t {
+fn sweepPid(name: []const u8) ?libc.pid_t {
     if (!std.mem.startsWith(u8, name, "pardes-") or !std.mem.endsWith(u8, name, ".sock")) return null;
     const digits = name["pardes-".len .. name.len - ".sock".len];
     if (digits.len == 0) return null;
@@ -218,17 +214,22 @@ fn sweep(dir: [:0]const u8) void {
 
 /// Bind and listen so nested instances can find us; -1 if anything fails, and
 /// a pardes without a socket is simply one whose children open their own UI.
+/// The path is always this process's own, so nobody outside holds a buffer of
+/// it — the shells each kept one and passed it back to be unlinked, which is a
+/// way for the two spellings to go out of step and for no other reason.
 ///
 /// CLOEXEC matters more here than on any other fd in the program: pane shells
 /// are forked with forkpty and inherit everything open, and an orphaned bash
 /// holding this one would keep the socket bound long after we exit — the same
 /// shape as the inherited lock fd that once held a flock forever.
-pub fn listenAt(path: [:0]const u8) c_int {
+pub fn listen() c_int {
     if (comptime builtin.os.tag != .linux) return -1;
     var dir_buf: [108:0]u8 = undefined;
     const dir = socketDir(&dir_buf) orelse return -1;
     if (!ensureSocketDir(dir)) return -1;
     sweep(dir);
+    var path_buf: [108]u8 = undefined;
+    const path = socketPath(&path_buf, libc.getpid()) orelse return -1;
     var addr: libc.sockaddr.un = .{ .path = @splat(0) };
     if (path.len + 1 > addr.path.len) return -1;
     @memcpy(addr.path[0 .. path.len + 1], path[0 .. path.len + 1]);
@@ -249,6 +250,17 @@ pub fn listenAt(path: [:0]const u8) c_int {
     return fd;
 }
 
+/// Close the listener and take its file away. Guarded on the fd rather than on
+/// the path, so a bind that FAILED cannot unlink a path this process never
+/// created; anything else is a no-op, which is what --nested and every
+/// non-linux build hand it.
+pub fn unlisten(fd: c_int) void {
+    if (fd < 0) return;
+    _ = libc.close(fd);
+    var path_buf: [108]u8 = undefined;
+    if (socketPath(&path_buf, libc.getpid())) |path| _ = libc.unlink(path);
+}
+
 /// Block until a nested instance sends a `Look` line, and return it inside
 /// `buf`. Null only when the listening fd itself is gone — teardown closed it,
 /// or it was never a socket — because anything else (EMFILE, ECONNABORTED)
@@ -282,11 +294,7 @@ pub fn acceptLine(fd: c_int, buf: []u8) ?[]const u8 {
         const tv: libc.timeval = .{ .sec = 1, .usec = 0 };
         _ = libc.setsockopt(conn, libc.SOL.SOCKET, libc.SO.RCVTIMEO, &tv, @sizeOf(libc.timeval));
         var len: usize = 0;
-        // ...and a cap on the reads themselves, because the timeout is PER
-        // read and a peer dribbling one byte under it would otherwise stretch
-        // to buf.len seconds. One line is one or two reads.
-        var reads: usize = 0;
-        while (len < buf.len and reads < 64) : (reads += 1) {
+        while (len < buf.len) {
             const n = libc.read(conn, buf.ptr + len, buf.len - len);
             if (n < 0 and libc.errno(n) == .INTR) continue;
             if (n <= 0) break; // EOF, or the receive timeout expired
@@ -307,6 +315,21 @@ pub fn acceptLine(fd: c_int, buf: []u8) ?[]const u8 {
 
 test "socket path: XDG first, then a private dir under HOME, never /tmp" {
     var buf: [108]u8 = undefined;
+    // The environment is process-wide and every test in this binary shares it.
+    // The last case below reaches the "no directory at all" branch by blanking
+    // both variables, and without this every later test ran without a HOME.
+    var xdg_buf: [4096:0]u8 = undefined;
+    var home_buf: [4096:0]u8 = undefined;
+    const xdg0 = if (libc.getenv("XDG_RUNTIME_DIR")) |v| std.fmt.bufPrintSentinel(&xdg_buf, "{s}", .{std.mem.span(v)}, 0) catch null else null;
+    const home0 = if (libc.getenv("HOME")) |v| std.fmt.bufPrintSentinel(&home_buf, "{s}", .{std.mem.span(v)}, 0) catch null else null;
+    defer {
+        if (xdg0) |v| {
+            _ = setenv("XDG_RUNTIME_DIR", v, 1);
+        } else _ = unsetenv("XDG_RUNTIME_DIR");
+        if (home0) |v| {
+            _ = setenv("HOME", v, 1);
+        } else _ = unsetenv("HOME");
+    }
     _ = setenv("XDG_RUNTIME_DIR", "/run/user/1000", 1);
     try std.testing.expectEqualStrings("/run/user/1000/pardes-4242.sock", socketPath(&buf, 4242).?);
     _ = unsetenv("XDG_RUNTIME_DIR");