From bc8ed5eb8ae2cc2936f91cf9d19f0fd1b3bafd36 Mon Sep 17 00:00:00 2001
From: modeco80 <lily.modeco80@protonmail.ch>
Date: Sat, 3 Feb 2024 19:19:10 -0500
Subject: [PATCH] migrate all manual closing to new UniqueFd

Makes things cleaner.
---
 src/EventLoop.cpp |  8 ++-----
 src/EventLoop.hpp |  3 ++-
 src/Process.cpp   |  8 +++----
 src/Process.hpp   |  3 ++-
 src/Timer.cpp     |  8 +++----
 src/Timer.hpp     |  3 ++-
 src/Types.hpp     | 57 +++++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 71 insertions(+), 19 deletions(-)

diff --git a/src/EventLoop.cpp b/src/EventLoop.cpp
index 32274cc..30c796a 100644
--- a/src/EventLoop.cpp
+++ b/src/EventLoop.cpp
@@ -13,12 +13,8 @@ namespace nanosm {
 		}
 	}
 
-	EventLoop::~EventLoop() {
-		if(epollFd == -1) {
-			close(epollFd);
-		}
-	}
-
+	EventLoop::~EventLoop() = default;
+	
 	void EventLoop::Post(PostFn func) {
 		if(!func)
 			return;
diff --git a/src/EventLoop.hpp b/src/EventLoop.hpp
index f65f9d1..1e4b892 100644
--- a/src/EventLoop.hpp
+++ b/src/EventLoop.hpp
@@ -62,13 +62,14 @@ namespace nanosm {
 		void Stop();
 
 	   private:
-		int epollFd {};
+		nanosm::UniqueFd epollFd {};
 		bool shouldStop { false };
 
 		/// All tracked IO objects.
 		std::set<std::shared_ptr<IoObject>> ioObjects {};
 
 		/// FIFO queue of functions to run semi-asynchronously
+		/// One function is ran every eventloop tick.
 		std::deque<PostFn> postCallbacks {};
 	};
 
diff --git a/src/Process.cpp b/src/Process.cpp
index 544a1b1..745ec10 100644
--- a/src/Process.cpp
+++ b/src/Process.cpp
@@ -36,7 +36,7 @@ namespace nanosm {
 	void Process::Respawn() {
 		pid = Clone3({
 		.flags = CLONE_PIDFD,
-		.pidfd = std::bit_cast<u64>(&pidFd),
+		.pidfd = std::bit_cast<u64>(pidFd.GetFDMutPtr()),
 		.exit_signal = SIGCHLD,
 		});
 
@@ -95,10 +95,8 @@ namespace nanosm {
 	}
 
 	void Process::Reset() {
-		if(pidFd != -1) {
-			close(pidFd);
-			pidFd = -1;
-		}
+		if(pidFd.Valid())
+			pidFd.Reset();
 
 		pid = -1;
 		siginfo = {};
diff --git a/src/Process.hpp b/src/Process.hpp
index e153293..e18b624 100644
--- a/src/Process.hpp
+++ b/src/Process.hpp
@@ -4,6 +4,7 @@
 #include <sys/wait.h>
 
 #include "EventLoop.hpp"
+#include "Types.hpp"
 
 namespace nanosm {
 
@@ -36,7 +37,7 @@ namespace nanosm {
 	   private:
 		void Reset();
 
-		int pidFd { -1 };
+		nanosm::UniqueFd pidFd { -1 };
 		pid_t pid { -1 };
 		siginfo_t siginfo {};
 		std::string commLine;
diff --git a/src/Timer.cpp b/src/Timer.cpp
index 922b32b..f8f5e0f 100644
--- a/src/Timer.cpp
+++ b/src/Timer.cpp
@@ -12,13 +12,11 @@ namespace nanosm {
 		}
 	}
 
-	Timer::~Timer() {
-		Reset();
-	}
+	Timer::~Timer() = default;
 
 	void Timer::Reset() {
-		if(timerFd != -1)
-			close(timerFd);
+		if(timerFd.Valid())
+			timerFd.Reset();
 	}
 
 	int Timer::GetFD() const {
diff --git a/src/Timer.hpp b/src/Timer.hpp
index 1de9865..28f3378 100644
--- a/src/Timer.hpp
+++ b/src/Timer.hpp
@@ -1,5 +1,6 @@
 #pragma once
 #include "EventLoop.hpp"
+#include "Types.hpp"
 
 namespace nanosm {
 
@@ -23,7 +24,7 @@ namespace nanosm {
 		void Arm(u32 durationSeconds);
 
 	   private:
-		int timerFd { -1 };
+		nanosm::UniqueFd timerFd { -1 };
 		std::function<void()> cb;
 	};
 
diff --git a/src/Types.hpp b/src/Types.hpp
index 152192b..42aefad 100644
--- a/src/Types.hpp
+++ b/src/Types.hpp
@@ -46,4 +46,61 @@ namespace nanosm {
 		usize size {};
 	};
 
+	/// A "smart pointer" for file descriptors.
+	struct UniqueFd {
+		constexpr UniqueFd()
+			: UniqueFd(-1) {}
+
+		constexpr explicit UniqueFd(int fd)
+			: fd(fd) {
+		}
+
+		constexpr UniqueFd(UniqueFd&& from) {
+			fd = std::move(from.fd);
+			from.fd = -1;
+		}
+
+		constexpr ~UniqueFd() {
+			Reset();
+		}
+
+		/// Assigns a new file descriptor to this UniqueFd,
+		/// closing the old one.
+		constexpr UniqueFd& operator=(int fd) {
+			Reset(fd);
+			return *this;
+		}
+
+		constexpr int GetFD() const { return fd; }
+
+		/// Gets a mutable pointer to the file descriptor value.
+		///
+		/// You should only use this in clone3() or such system
+		/// calls that expect an address to a value to populate
+		/// with a known-valid/good file descriptor. Otherwise,
+		/// this function is very unsafe and a easy way to FD leak.
+		int* GetFDMutPtr() { return &fd; }
+
+		constexpr operator int() const {
+			return GetFD();
+		}
+
+		constexpr bool Valid() const {
+			return fd != -1;
+		}
+
+		constexpr void Reset(int newFd = -1) {
+			// If a previously valid file descriptor was assigned,
+			// then we have to close it to avoid leaking file
+			// descriptors. Do so here.
+			if(Valid())
+				close(fd);
+
+			fd = newFd;
+		}
+
+	   private:
+		int fd { -1 };
+	};
+
 } // namespace nanosm