make take_descriptor{,_array} exception-safe

5 files changed, 127 insertions, 12 deletions

diff --git a/libnvxx/nv_list.cc b/libnvxx/nv_list.cc
index c2b9372..b5e7b94 100644
--- a/libnvxx/nv_list.cc
+++ b/libnvxx/nv_list.cc
@@ -710,12 +710,13 @@ __nv_list::free_nvlist_array(std::string_view key)
  * descriptor operations
  */
 
-int
+nv_fd
 __nv_list::take_descriptor(std::string_view key)
 {
 	__throw_if_error();
 
-	return (::nvlist_take_descriptor(__m_nv, std::string(key).c_str()));
+	auto fd = ::nvlist_take_descriptor(__m_nv, std::string(key).c_str());
+	return (nv_fd(fd));
 }
 
 void
@@ -805,17 +806,34 @@ __nv_list::free_descriptor_array(std::string_view key)
 	free_type(key, NV_TYPE_DESCRIPTOR_ARRAY);
 }
 
-std::vector<int>
+std::vector<nv_fd>
 __nv_list::take_descriptor_array(std::string_view key)
 {
 	__throw_if_error();
 
+	// define this here to avoid throwing after we call
+	// nvlist_take_descriptor_array().
+	auto ret = std::vector<nv_fd>{};
+
 	auto nitems = std::size_t{};
 	auto ptr = __ptr_guard(
 		::nvlist_take_descriptor_array(__m_nv,
 					       std::string(key).c_str(),
 					       &nitems));
-	return {ptr.__ptr, ptr.__ptr + nitems};
+	auto fds = std::span(ptr.__ptr, ptr.__ptr + nitems);
+
+	try {
+		ret.reserve(fds.size());
+		for (auto fd : fds)
+			ret.push_back(nv_fd(fd));
+
+		return ret;
+	} catch (...) {
+		// close all the fds we didn't manage to add to the vector
+		for (auto fd : fds.subspan(ret.size()))
+			(void)::close(fd);
+		throw;
+	}
 }
 
 /*
diff --git a/libnvxx/nvxx.3 b/libnvxx/nvxx.3
index 96cd514..970e707 100644
--- a/libnvxx/nvxx.3
+++ b/libnvxx/nvxx.3
@@ -16,6 +16,19 @@
 namespace bsd {
 
 // exposition only
+struct nv_fd {
+        explicit nv_fd(int fd) noexcept;
+        nv_fd(nv_fd &&other) noexcept;
+        nv_fd(nv_fd const &) = delete;
+
+        nv_fd& operator=(nv_fd &&other) noexcept;
+        nv_fd& operator=(nv_fd const &) = delete;
+
+        int get() const;
+        int release() &&;
+};
+
+// exposition only
 struct nv_error : std::runtime_error {
 };
 
@@ -146,7 +159,7 @@ struct nv_list : <unspecified> {
 	auto take_bool(std::string_view key) -> bool;
 	auto take_number(std::string_view key) -> std::uint64_t;
 	auto take_string(std::string_view key) -> std::string;
-	auto take_descriptor(std::string_view key) -> int;
+	auto take_descriptor(std::string_view key) -> nv_fd;
 	auto take_nvlist(std::string_view key) -> nv_list;
 
 	void move_bool_array(std::string_view key, std::span<bool>);
@@ -196,7 +209,7 @@ struct nv_list : <unspecified> {
 	auto take_number_array(std::string_view key) -> container-type<std::uint64_t>;
 	auto take_string_array(std::string_view key) -> container-type<std::string>;
 	auto take_nvlist_array(std::string_view key) -> container-type<nv_list>;
-	auto take_descriptor_array(std::string_view __key) -> container-type<int>;
+	auto take_descriptor_array(std::string_view __key) -> container-type<nv_fd>;
 	auto take_binary(std::string_view key) -> container-type<std::byte>;
 };
 
@@ -313,6 +326,34 @@ exception.
 The
 .Va nv_error_state::error
 member variable may then be used to determine which specific error occurred.
+.Sh THE NV_FD TYPE
+The C++ library uses a type called
+.Vt bsd::nv_fd
+for functions which take or return file descriptors.
+An
+.Vt nv_fd
+may be created from a file descriptor, at which point it takes ownership of the
+file descriptor and will close it when destructed.
+An
+.Vt nv_fd
+may be move-initialized and move-assigned, but may not be copied.
+.Pp
+To retrieve the file descriptor stored by an
+.Vt nv_fd
+without taking ownership, use the
+.Fn get
+member function.
+To retrieve the file descriptor and take ownership of it, use the
+.Fn release
+member function, which leaves the
+.Vt nv_fd
+in a moved-from state.
+.Pp
+A moved-from
+.Vt nv_fd
+may be assigned to, but any attempt to retrieve the store file descriptor will
+throw an exception of type
+.Vt std::logic_error .
 .Sh CONST_NV_LIST OPERATIONS
 The
 .Fn dump
diff --git a/libnvxx/nvxx.h b/libnvxx/nvxx.h
index e404cfe..d2f8608 100644
--- a/libnvxx/nvxx.h
+++ b/libnvxx/nvxx.h
@@ -21,6 +21,8 @@
 #include <stdexcept>
 #include <format>
 
+#include <unistd.h>
+
 #include "nvxx_util.h"
 #include "nvxx_base.h"
 #include "nvxx_iterator.h"
diff --git a/libnvxx/nvxx_base.h b/libnvxx/nvxx_base.h
index da2978b..c925c2c 100644
--- a/libnvxx/nvxx_base.h
+++ b/libnvxx/nvxx_base.h
@@ -167,8 +167,6 @@ struct __const_nv_list : virtual __nv_list_base {
 	[[nodiscard]] bool exists_type(std::string_view __key,
 				       int __type) const;
 
-	// TODO: exists_type()
-
 	/* exists */
 
 	[[nodiscard]] bool exists_null(std::string_view __key) const;
@@ -289,7 +287,7 @@ struct __nv_list : virtual __nv_list_base {
 		-> std::uint64_t;
 	[[nodiscard]] auto take_string(std::string_view __key) -> std::string;
 	[[nodiscard]] auto take_nvlist(std::string_view __key) -> nv_list;
-	[[nodiscard]] auto take_descriptor(std::string_view __key) -> int;
+	[[nodiscard]] auto take_descriptor(std::string_view __key) -> nv_fd;
 	[[nodiscard]] auto take_binary(std::string_view __key)
 		-> std::vector<std::byte>;
 
@@ -307,7 +305,7 @@ struct __nv_list : virtual __nv_list_base {
 	take_nvlist_array(std::string_view __key) -> std::vector<nv_list>;
 
 	[[nodiscard]] auto
-	take_descriptor_array(std::string_view __key) -> std::vector<int>;
+	take_descriptor_array(std::string_view __key) -> std::vector<nv_fd>;
 
 	/* move */
 
diff --git a/libnvxx/nvxx_util.h b/libnvxx/nvxx_util.h
index 602c30a..f50b0d2 100644
--- a/libnvxx/nvxx_util.h
+++ b/libnvxx/nvxx_util.h
@@ -12,8 +12,63 @@
 
 // Some useful helper types.
 
-namespace bsd::__detail {
+namespace bsd {
 
+/*
+ * An RAII guard for a file descriptor.
+ */
+struct nv_fd {
+	explicit nv_fd(int __fd) noexcept
+		: __m_fd(__fd)
+	{
+	}
+
+	nv_fd(nv_fd const &) = delete;
+
+	nv_fd(nv_fd &&__other) noexcept
+		: __m_fd(std::exchange(__other.__m_fd, -1))
+	{
+	}
+
+	nv_fd& operator=(nv_fd const &) = delete;
+
+	nv_fd& operator=(nv_fd &&__other) noexcept {
+		if (this != &__other) {
+			if (__m_fd != -1)
+				(void)::close(__m_fd);
+			__m_fd = std::exchange(__other.__m_fd, -1);
+		}
+		return (*this);
+	}
+
+	~nv_fd() {
+		if (__m_fd != -1)
+			(void)::close(__m_fd);
+	}
+
+	int get() const {
+		if (__m_fd == -1)
+			throw std::logic_error("attempt to access a "
+					       "closed nv_fd");
+		return (__m_fd);
+	}
+
+	int release() && {
+		if (__m_fd == -1)
+			throw std::logic_error("attempt to access a "
+					       "closed nv_fd");
+		return (std::exchange(__m_fd, -1));
+	}
+
+private:
+	int __m_fd = -1;
+};
+
+namespace __detail {
+
+/*
+ * An RAII guard for a C pointer.
+ */
 template<typename _T>
 struct __ptr_guard {
 	__ptr_guard(_T *__ptr_) : __ptr(__ptr_) {}
@@ -30,6 +85,7 @@ auto construct = std::views::transform([] (auto &&value) {
 	return (T(std::forward<decltype(value)>(value)));
 });
 
-} // namespace bsd::__detail
+} // namespace __detail
+} // namespace bsd
 
 #endif	/* !_NVXX_UTIL_H_INCLUDED */